SCSI Data Integrity Support
PATCH 1/1 SCSI Data Integrity Support
These patches are against Linus' current git tree which now has all of the block layer data integrity code + the preparatory SCSI bits.
Changes since last post:
- Fixed a bug in error handling when HBA detected an integrity error.
- Introduced prot_type in scsi_cmnd which informs the HBA of the target type. This is necessary because the HBA needs to know which fields in the protection information to check and that depends on the target type.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering
PATCH 1/1 scsi: Command protection operation
Controllers that support DMA of protection information must be told explicitly how to handle the I/O. The controller has no knowledge of the protection capabilities of the target device so this information must be passed in the scsi_cmnd.
- The protection operation tells the HBA whether to generate, strip or verify protection information.
- The protection type tells the HBA which layout the target is formatted with. This is necessary because the controller must be able to correctly interpret the included protection information in order to verify it.
- When a scsi_cmnd is reused for error handling the protection operation must be cleared and saved while error handling is in progress.
- prot_op and prot_type are placed in an existing hole in scsi_cmnd and don't cause the structure to grow.
---
3 files changed, 70 insertions(+) drivers/scsi/scsi_error.c | 3 ++ include/scsi/scsi_cmnd.h | 66 +++++++++++++++++++++++++++++++++++++++++++++ include/scsi/scsi_eh.h | 1
drivers/scsi/scsi_error.c +3 -0
@@ -664,7 +664,9 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd ses->sdb = scmd->sdb; ses->next_rq = scmd->request->next_rq; ses->result = scmd->result; + ses->prot_op = scmd->prot_op; + scmd->prot_op =SCSI_PROT_NORMAL ; scmd->cmnd = ses->eh_cmnd;memset (scmd->cmnd,0 ,BLK_MAX_CDB );memset (&scmd->sdb,0 ,sizeof (scmd->sdb));
@@ -722,6 +724,7 @@ void scsi_eh_restore_cmnd(struct scsi_cm scmd->sdb = ses->sdb; scmd->request->next_rq = ses->next_rq; scmd->result = ses->result; + scmd->prot_op = ses->prot_op; }EXPORT_SYMBOL (scsi_eh_restore_cmnd);
include/scsi/scsi_cmnd.h +66 -0
76 lines not on this page · raw message
include/scsi/scsi_eh.h +1 -0
@@ -75,6 +75,7 @@ struct scsi_eh_save {int result;enum dma_data_direction data_direction;unsigned char cmd_len; +unsigned char prot_op;unsigned char *cmnd;struct scsi_data_buffer sdb;struct request *next_rq;
PATCH 1/1 scsi: Host protection capabilities
In reply to Martin K. PetersenControllers that support protection information must indicate this to the SCSI midlayer so that the ULD can prepare scsi_cmnds accordingly.
This patch implements a host mask and various types of protection:
- DIF Type 1-3 (between HBA and disk)
- DIX Type 0-3 (between OS and HBA)
The patch also allows the HBA to set the guard type to something different than the T10-mandated CRC.
---
2 files changed, 88 insertions(+)
drivers/scsi/scsi_sysfs.c | 4 ++include/scsi/scsi_host.h | 84 +++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/scsi_sysfs.c +4 -0
@@ -249,6 +249,8 @@ shost_rd_attr(can_queue, "%hd\n");shost_rd_attr (can_queue,"%hd\n" );shost_rd_attr (sg_tablesize,"%hu\n" );shost_rd_attr (unchecked_isa_dma,"%d\n" ); +shost_rd_attr (prot_capabilities,"%u\n" ); +shost_rd_attr (prot_guard_type,"%hd\n" );shost_rd_attr2 (proc_name, hostt->proc_name,"%s\n" );static struct attribute *scsi_sysfs_shost_attrs[] = {
@@ -263,6 +265,8 @@ static struct attribute *scsi_sysfs_shos &dev_attr_hstate.attr, &dev_attr_supported_mode.attr, &dev_attr_active_mode.attr, + &dev_attr_prot_capabilities.attr, + &dev_attr_prot_guard_type.attr, NULL };
include/scsi/scsi_host.h +84 -0
96 lines not on this page · raw message
PATCH 1/1 sd: Identify DIF protection type and application tag ownership
In reply to Martin K. PetersenIf a disk is formatted with protection information (Inquiry bit PROTECT=1) it is required to support Read Capacity(16). Force use of the 16-bit command in this case and extract the P_TYPE field which indicates whether the disk is formatted using DIF Type 1, 2 or 3.
The ATO (App Tag Own) bit in the Control Mode Page indicates whether the storage device or the initiator own the contents of the DIF application tag.
---
2 files changed, 141 insertions(+), 4 deletions(-)
drivers/scsi/sd.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++--drivers/scsi/sd.h | 23 +++++++++
drivers/scsi/sd.c +118 -4
@@ -234,6 +234,24 @@ sd_show_allow_restart(struct device *devreturn snprintf (buf,40 ,"%d\n" , sdkp->device->allow_restart); } +static ssize_t +sd_show_protection_type (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct scsi_disk *sdkp =to_scsi_disk (dev); + +return snprintf (buf,20 ,"%u\n" , sdkp->protection_type); +} + +static ssize_t +sd_show_app_tag_own (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct scsi_disk *sdkp =to_scsi_disk (dev); + +return snprintf (buf,20 ,"%u\n" , sdkp->ATO); +} +static struct device_attribute sd_disk_attrs[] = {__ATTR (cache_type,S_IRUGO |S_IWUSR , sd_show_cache_type, sd_store_cache_type),
@@ -242,6 +260,8 @@ static struct device_attribute sd_disk_a sd_store_allow_restart),__ATTR (manage_start_stop,S_IRUGO |S_IWUSR , sd_show_manage_start_stop, sd_store_manage_start_stop), +__ATTR (protection_type,S_IRUGO , sd_show_protection_type,NULL ), +__ATTR (app_tag_own,S_IRUGO , sd_show_app_tag_own,NULL ), __ATTR_NULL, };
@@ -1165,6 +1185,49 @@ sd_spinup_disk(struct scsi_disk *sdkp) } } + +/* +* Determine whether disk supports Data Integrity Field. +*/ +void sd_read_protection_type (struct scsi_disk *sdkp,unsigned char *buffer) +{ +struct scsi_device *sdp = sdkp->device; +u8 type; + +if (scsi_device_protection (sdp) ==0 || (buffer[12 ] &1 ) ==0 ) + type =0 ; +else + type = ((buffer[12 ] >>1 ) &7 ) +1 ;/* P_TYPE 0 = Type 1 */ + +switch (type) { +case SD_DIF_TYPE0_PROTECTION : + sdkp->protection_type =0 ; +break ; + +case SD_DIF_TYPE1_PROTECTION : +case SD_DIF_TYPE3_PROTECTION : + sdkp->protection_type = type; +break ; + +case SD_DIF_TYPE2_PROTECTION : +sd_printk (KERN_ERR , sdkp,"formatted with DIF Type 2 " \ +"protection which is currently unsupported. " \ +"Disabling disk!\n" ); + goto disable; + +default : +sd_printk (KERN_ERR , sdkp,"formatted with unknown " \ +"protection type %d. Disabling disk!\n" , type); + goto disable; + } + +return ; + +disable: + sdkp->protection_type =0 ; + sdkp->capacity =0 ; +} +/* * read disk capacity */
@@ -1174,7 +1237,8 @@ sd_read_capacity(struct scsi_disk *sdkp,unsigned char cmd[16 ];int the_result, retries;int sector_size =0 ; -int longrc =0 ; +/* Force READ CAPACITY(16) when PROTECT=1 */ +int longrc =scsi_device_protection (sdkp->device) ?1 :0 ;struct scsi_sense_hdr sshdr;int sense_valid =0 ;struct scsi_device *sdp = sdkp->device;
@@ -1186,8 +1250,8 @@ repeat:memset ((void *) cmd,0 ,16 ); cmd[0 ] =SERVICE_ACTION_IN ; cmd[1 ] =SAI_READ_CAPACITY_16 ; - cmd[13 ] =12 ; -memset ((void *) buffer,0 ,12 ); + cmd[13 ] =13 ; +memset ((void *) buffer,0 ,13 ); }else { cmd[0 ] =READ_CAPACITY ;memset ((void *) &cmd[1 ],0 ,9 );
@@ -1195,7 +1259,7 @@ repeat: } the_result =scsi_execute_req (sdp, cmd,DMA_FROM_DEVICE , - buffer, longrc ?12 :8 , &sshdr, + buffer, longrc ?13 :8 , &sshdr,SD_TIMEOUT ,SD_MAX_RETRIES );if (media_not_present (sdkp, &sshdr))
@@ -1270,6 +1334,8 @@ repeat: sector_size = (buffer[8 ] <<24 ) | (buffer[9 ] <<16 ) | (buffer[10 ] <<8 ) | buffer[11 ]; + +sd_read_protection_type (sdkp, buffer); }/* Some devices return the total number of sectors, not the
@@ -1531,6 +1597,52 @@ defaults: sdkp->DPOFUA =0 ; } +/* +* The ATO bit indicates whether the DIF application tag is available +* for use by the operating system. +*/ +void sd_read_app_tag_own (struct scsi_disk *sdkp,unsigned char *buffer) +{ +int res, offset; +struct scsi_device *sdp = sdkp->device; +struct scsi_mode_data data; +struct scsi_sense_hdr sshdr; + +if (sdp->type !=TYPE_DISK ) +return ; + +if (sdkp->protection_type ==0 ) +return ; + + res =scsi_mode_sense (sdp,1 ,0x0a , buffer,36 ,SD_TIMEOUT , +SD_MAX_RETRIES , &data, &sshdr); + +if (!scsi_status_is_good (res) || !data.header_length || + data.length <6 ) { +sd_printk (KERN_WARNING , sdkp, +"getting Control mode page failed, assume no ATO\n" ); + +if (scsi_sense_valid (&sshdr)) +sd_print_sense_hdr (sdkp, &sshdr); + +return ; + } + + offset = data.header_length + data.block_descriptor_length; + +if ((buffer[offset] &0x3f ) !=0x0a ) { +sd_printk (KERN_ERR , sdkp,"ATO Got wrong page\n" ); +return ; + } + +if ((buffer[offset +5 ] &0x80 ) ==0 ) +return ; + + sdkp->ATO =1 ; + +return ; +} +/** * sd_revalidate_disk - called the first time a new disk is seen, * performs disk spin up, read_capacity, etc.
@@ -1567,6 +1679,7 @@ static int sd_revalidate_disk(struct gen sdkp->write_prot =0 ; sdkp->WCE =0 ; sdkp->RCD =0 ; + sdkp->ATO =0 ;sd_spinup_disk (sdkp);
@@ -1578,6 +1691,7 @@ static int sd_revalidate_disk(struct gensd_read_capacity (sdkp, buffer);sd_read_write_protect_flag (sdkp, buffer);sd_read_cache_type (sdkp, buffer); +sd_read_app_tag_own (sdkp, buffer); }/*
drivers/scsi/sd.h +23 -0
@@ -41,7 +41,9 @@ struct scsi_disk {u32 index;u8 media_present;u8 write_prot; +u8 protection_type;/* Data Integrity Field */ unsigned previous_state : 1; + unsigned ATO : 1; /* state of disk ATO bit */ unsigned WCE : 1; /* state of disk WCE bit */ unsigned RCD : 1; /* state of disk RCD bit, unused */ unsigned DPOFUA : 1; /* state of disk DPOFUA bit */
@@ -59,4 +61,25 @@ static inline struct scsi_disk *scsi_dis (sdsk)->disk->disk_name, ##a) : \ sdev_printk(prefix, (sdsk)->device, fmt, ##a) +/* +* A DIF-capable target device can be formatted with different +* protection schemes. Currently 0 through 3 are defined: +* +* Type 0 is regular (unprotected) I/O +* +* Type 1 defines the contents of the guard and reference tags +* +* Type 2 defines the contents of the guard and reference tags and +* uses 32-byte commands to seed the latter +* +* Type 3 defines the contents of the guard tag only +*/ + +enum sd_dif_target_protection_types { +SD_DIF_TYPE0_PROTECTION =0x0 , +SD_DIF_TYPE1_PROTECTION =0x1 , +SD_DIF_TYPE2_PROTECTION =0x2 , +SD_DIF_TYPE3_PROTECTION =0x3 , +}; +#endif /* _SCSI_DISK_H */
PATCH 1/1 scsi: Do not retry a request whose data integrity check failed
In reply to Martin K. PetersenIf initiator or target reject the I/O due to DIF errors there is no point in retrying.
---
2 files changed, 10 insertions(+), 2 deletions(-) drivers/scsi/scsi_error.c | 3 +++ drivers/scsi/scsi_lib.c | 9 +++++++--
drivers/scsi/scsi_error.c +3 -0
@@ -344,6 +344,9 @@ static int scsi_check_sense(struct scsi_return /* soft_error */ SUCCESS ;case ABORTED_COMMAND : +if (sshdr.asc ==0x10 )/* DIF */ +return SUCCESS ; +return NEEDS_RETRY ;case NOT_READY :case UNIT_ATTENTION :
drivers/scsi/scsi_lib.c +7 -2
@@ -950,9 +950,14 @@ void scsi_io_completion(struct scsi_cmnd* 6-byte command. */ scsi_requeue_command (q, cmd); -return ; - }else { + }else if (sshdr.asc ==0x10 )/* DIX */ +scsi_end_request (cmd, -EIO , this_count,0 ); +else scsi_end_request (cmd, -EIO , this_count,1 ); +return ; +case ABORTED_COMMAND : +if (sshdr.asc ==0x10 ) {/* DIF */ +scsi_end_request (cmd, -EIO , this_count,0 );return ; }break ;
PATCH 1/1 scsi: Support devices with protection information
In reply to Martin K. PetersenImplement support for DMA of protection information for devices that are data integrity capable.
- Add support for mapping an extra scatter-gather list containing the protection information.
- Handle protection scsi_data_buffer in command allocation.
- Preallocate protection scsi_data_buffer in command freelist if host is DIX (integrity DMA) capable.
- Accessor function for checking whether a device has protection enabled.
---
7 files changed, 97 insertions(+), 11 deletions(-) drivers/scsi/scsi.c | 47 ++++++++++++++++++++++++++++++++++++++----- drivers/scsi/scsi_error.c | 2 - drivers/scsi/scsi_lib.c | 27 ++++++++++++++++++++++-- drivers/scsi/scsi_priv.h | 1 drivers/scsi/scsi_tgt_lib.c | 2 - include/scsi/scsi_cmnd.h | 24 ++++++++++++++++++++- include/scsi/scsi_device.h | 5 ++++
drivers/scsi/scsi.c +42 -5
@@ -199,24 +199,60 @@ scsi_pool_free_command(struct scsi_host_ {kmem_cache_free (pool->sense_slab, cmd->sense_buffer);kmem_cache_free (pool->cmd_slab, cmd); + +if (cmd->prot_sdb) +kmem_cache_free (scsi_sdb_cache, cmd->prot_sdb); +} + +/** +* scsi_host_alloc_command - internal function to allocate command +* @shost: SCSI host whose pool to allocate from +* @gfp_mask: mask for the allocation +* +* Returns a fully allocated command with sense buffer and protection +* data buffer (where applicable) or NULL on failure +*/ +static struct scsi_cmnd * +scsi_host_alloc_command (struct Scsi_Host *shost,gfp_t gfp_mask) +{ +struct scsi_cmnd *cmd; + + cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); +if (!cmd) +return NULL ; + +if (scsi_host_get_prot (shost) >=SHOST_DIX_TYPE0_PROTECTION ) { + cmd->prot_sdb =kmem_cache_zalloc (scsi_sdb_cache, gfp_mask); + +if (!cmd->prot_sdb) { +scsi_pool_free_command (shost->cmd_pool, cmd); +return NULL ; + } + } + +return cmd; }/** * __scsi_get_command - Allocate a struct scsi_cmnd * @shost: host to transmit command * @gfp_mask: allocation mask +* @protection: allocate protection sdb * * Description: allocate a struct scsi_cmd from host's slab, recycling from the * host's free_list if necessary. */ -struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *shost,gfp_t gfp_mask) +struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *shost,gfp_t gfp_mask,int protection) {struct scsi_cmnd *cmd;unsigned char *buf; cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); -if (unlikely (!cmd)) { +if (protection) + cmd->prot_sdb =kmem_cache_zalloc (scsi_sdb_cache, gfp_mask); + +if (unlikely (!cmd) ||unlikely (protection && cmd->prot_sdb ==NULL )) {unsigned long flags;spin_lock_irqsave (&shost->free_list_lock, flags);
@@ -242,10 +278,11 @@ EXPORT_SYMBOL_GPL(__scsi_get_command);* scsi_get_command - Allocate and setup a scsi command block * @dev: parent scsi device * @gfp_mask: allocator flags +* @protection: allocate protection sdb * * Returns: The allocated scsi command structure. */ -struct scsi_cmnd *scsi_get_command (struct scsi_device *dev,gfp_t gfp_mask) +struct scsi_cmnd *scsi_get_command (struct scsi_device *dev,gfp_t gfp_mask,int protection) {struct scsi_cmnd *cmd;
@@ -253,7 +290,7 @@ struct scsi_cmnd *scsi_get_command(strucif (!get_device (&dev->sdev_gendev))return NULL ; - cmd =__scsi_get_command (dev->host, gfp_mask); + cmd =__scsi_get_command (dev->host, gfp_mask, protection);if (likely (cmd !=NULL )) {unsigned long flags;
@@ -457,7 +494,7 @@ int scsi_setup_command_freelist(struct S/* * Get one backup command for this host. */ - cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); + cmd =scsi_host_alloc_command (shost, gfp_mask);if (!cmd) {scsi_put_host_cmd_pool (gfp_mask); shost->cmd_pool =NULL ;
drivers/scsi/scsi_error.c +1 -1
@@ -1780,7 +1780,7 @@ intint scsi_reset_provider (struct scsi_device *dev,int flag) { -struct scsi_cmnd *scmd =scsi_get_command (dev,GFP_KERNEL ); +struct scsi_cmnd *scmd =scsi_get_command (dev,GFP_KERNEL ,0 );struct Scsi_Host *shost = dev->host;struct request req;unsigned long flags;
drivers/scsi/scsi_lib.c +25 -2
@@ -65,7 +65,7 @@ static struct scsi_host_sg_pool scsi_sg_ };#undef SP -static struct kmem_cache *scsi_sdb_cache; +struct kmem_cache *scsi_sdb_cache;static void scsi_run_queue (struct request_queue *q);
@@ -787,6 +787,9 @@ void scsi_release_buffers(struct scsi_cmkmem_cache_free (scsi_sdb_cache, bidi_sdb); cmd->request->next_rq->special =NULL ; } + +if (scsi_prot_sg_count (cmd)) +scsi_free_sgtable (cmd->prot_sdb); }EXPORT_SYMBOL (scsi_release_buffers);
@@ -1072,6 +1075,26 @@ int scsi_init_io(struct scsi_cmnd *cmd, goto err_exit; } +if (blk_integrity_rq (cmd->request)) { +struct scsi_data_buffer *prot_sdb = cmd->prot_sdb; +int ivecs, count; + +BUG_ON (prot_sdb ==NULL ); + ivecs =blk_rq_count_integrity_sg (cmd->request); + +if (scsi_alloc_sgtable (prot_sdb, ivecs, gfp_mask)) { + error =BLKPREP_DEFER ; + goto err_exit; + } + + count =blk_rq_map_integrity_sg (cmd->request, + prot_sdb->table.sgl); +BUG_ON (unlikely (count > ivecs)); + + cmd->prot_sdb = prot_sdb; + cmd->prot_sdb->table.nents = count; + } +return BLKPREP_OK ; err_exit:
@@ -1091,7 +1114,7 @@ static struct scsi_cmnd *scsi_get_cmd_frstruct scsi_cmnd *cmd;if (!req->special) { - cmd =scsi_get_command (sdev,GFP_ATOMIC ); + cmd =scsi_get_command (sdev,GFP_ATOMIC ,blk_integrity_rq (req));if (unlikely (!cmd))return NULL ; req->special = cmd;
drivers/scsi/scsi_priv.h +1 -0
@@ -77,6 +77,7 @@ struct request_queue;struct request_queue ;struct request ;extern int scsi_prep_fn (struct request_queue *,struct request *); +extern struct kmem_cache *scsi_sdb_cache;/* scsi_proc.c */ #ifdef CONFIG_SCSI_PROC_FS
drivers/scsi/scsi_tgt_lib.c +1 -1
@@ -99,7 +99,7 @@ struct scsi_cmnd *scsi_host_get_command(if (!rq) goto free_tcmd; - cmd =__scsi_get_command (shost, gfp_mask); + cmd =__scsi_get_command (shost, gfp_mask,0 );if (!cmd) goto release_rq;
include/scsi/scsi_cmnd.h +22 -2
@@ -90,6 +90,8 @@ struct scsi_cmnd {/* These elements define the operation we ultimately want to perform */ struct scsi_data_buffer sdb; +struct scsi_data_buffer *prot_sdb; +unsigned underflow;/* Return error if less than this amount is transferred */
@@ -131,8 +133,8 @@ struct scsi_cmnd {unsigned char tag;/* SCSI-II queued command tag */ }; -extern struct scsi_cmnd *scsi_get_command (struct scsi_device *,gfp_t ); -extern struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *,gfp_t ); +extern struct scsi_cmnd *scsi_get_command (struct scsi_device *,gfp_t ,int ); +extern struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *,gfp_t ,int );extern void scsi_put_command (struct scsi_cmnd *);extern void __scsi_put_command (struct Scsi_Host *,struct scsi_cmnd *,struct device *);
@@ -274,4 +276,22 @@ static inline sector_t scsi_get_lba(stru return scmd->request->sector; } +static inline unsigned scsi_prot_sg_count (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.nents :0 ; +} + +static inline struct scatterlist *scsi_prot_sglist (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.sgl :NULL ; +} + +static inline struct scsi_data_buffer *scsi_prot (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb; +} + +#define scsi_for_each_prot_sg (cmd, sg, nseg, __i) \ + for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i) +#endif /* _SCSI_SCSI_CMND_H */
include/scsi/scsi_device.h +5 -0
@@ -416,6 +416,11 @@ static inline int scsi_device_enclosure(return sdev->inquiry[6 ] & (1 <<6 ); } +static inline int scsi_device_protection (struct scsi_device *sdev) +{ +return sdev->inquiry[5 ] & (1 <<0 ); +} +#define MODULE_ALIAS_SCSI_DEVICE (type) \ MODULE_ALIAS("scsi:t-" __stringify(type) "*")#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
James Bottomley
maintainerRe: [PATCH 3 of 6] scsi: Support devices with protection information
drivers/scsi/scsi.c scsi_pool_free_command(struct scsi_host_ · line 253
Quoted 11 lines · Martin K. Petersen
drivers/scsi/scsi.c +3
-2-struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *shost,gfp_t gfp_mask) +struct scsi_cmnd *__scsi_get_command (struct Scsi_Host *shost,gfp_t gfp_mask,int protection) {struct scsi_cmnd *cmd;unsigned char *buf; cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); -if (unlikely (!cmd)) { +if (protection) + cmd->prot_sdb =kmem_cache_zalloc (scsi_sdb_cache, gfp_mask);
Really, no. if cmd is NULL, this will cause an immediate oops.
Review
Quoted 2 lines
(quoted diff) +2
-0+ + if (unlikely(!cmd) || unlikely(protection && cmd->prot_sdb == NULL)) {
And here, too ...
drivers/scsi/scsi.c scsi_pool_free_command(struct scsi_host_ · line 258
Quoted 2 lines
drivers/scsi/scsi.c +0
-0unsigned long flags;spin_lock_irqsave (&shost->free_list_lock, flags);
In general, you're on the right lines, the problem is with this protection flag. To use the poll like this, the protection is going to have to be always on or always off. If you try and gate it by a flag, the command on the free list is going to lose the protection buffer eventually (because it can get allocated on low memory, but it won't necessarily be the command returned to the free list, and if the returned command is allocated without protection, we lose the buffer).
James
Re: [PATCH 3 of 6] scsi: Support devices with protection information
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"James" == James Bottomley <[email protected]> writes:
James> Really, no. if cmd is NULL, this will cause an immediate oops.
*blush*
James> In general, you're on the right lines, the problem is with this James> protection flag. To use the poll like this, the protection is James> going to have to be always on or always off. If you try and James> gate it by a flag, the command on the free list is going to James> lose the protection buffer eventually (because it can get James> allocated on low memory, but it won't necessarily be the James> command returned to the free list, and if the returned command James> is allocated without protection, we lose the buffer).
Good point. The per-command allocation flag was a poorly executed optimisation afterthought. The following patch is what I did originally...
---8<---
scsi: Support devices with protection information
Implement support for DMA of protection information for devices that are data integrity capable.
- Add support for mapping an extra scatter-gather list containing the protection information.
- Allocate protection scsi_data_buffer if host is DIX (integrity DMA) capable.
- Accessor function for checking whether a device has protection enabled.
---
drivers/scsi/scsi.c +34 -2
@@ -199,6 +199,38 @@ scsi_pool_free_command(struct scsi_host_ {kmem_cache_free (pool->sense_slab, cmd->sense_buffer);kmem_cache_free (pool->cmd_slab, cmd); + +if (cmd->prot_sdb) +kmem_cache_free (scsi_sdb_cache, cmd->prot_sdb); +} + +/** +* scsi_host_alloc_command - internal function to allocate command +* @shost: SCSI host whose pool to allocate from +* @gfp_mask: mask for the allocation +* +* Returns a fully allocated command with sense buffer and protection +* data buffer (where applicable) or NULL on failure +*/ +static struct scsi_cmnd * +scsi_host_alloc_command (struct Scsi_Host *shost,gfp_t gfp_mask) +{ +struct scsi_cmnd *cmd; + + cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); +if (!cmd) +return NULL ; + +if (scsi_host_get_prot (shost) >=SHOST_DIX_TYPE0_PROTECTION ) { + cmd->prot_sdb =kmem_cache_zalloc (scsi_sdb_cache, gfp_mask); + +if (!cmd->prot_sdb) { +scsi_pool_free_command (shost->cmd_pool, cmd); +return NULL ; + } + } + +return cmd; }/**
@@ -214,7 +246,7 @@ struct scsi_cmnd *__scsi_get_command(strstruct scsi_cmnd *cmd;unsigned char *buf; - cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); + cmd =scsi_host_alloc_command (shost, gfp_mask);if (unlikely (!cmd)) {unsigned long flags;
@@ -457,7 +489,7 @@ int scsi_setup_command_freelist(struct S/* * Get one backup command for this host. */ - cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); + cmd =scsi_host_alloc_command (shost, gfp_mask);if (!cmd) {scsi_put_host_cmd_pool (gfp_mask); shost->cmd_pool =NULL ;
drivers/scsi/scsi_lib.c +24 -1
@@ -65,7 +65,7 @@ static struct scsi_host_sg_pool scsi_sg_ };#undef SP -static struct kmem_cache *scsi_sdb_cache; +struct kmem_cache *scsi_sdb_cache;static void scsi_run_queue (struct request_queue *q);
@@ -787,6 +787,9 @@ void scsi_release_buffers(struct scsi_cmkmem_cache_free (scsi_sdb_cache, bidi_sdb); cmd->request->next_rq->special =NULL ; } + +if (scsi_prot_sg_count (cmd)) +scsi_free_sgtable (cmd->prot_sdb); }EXPORT_SYMBOL (scsi_release_buffers);
@@ -1070,6 +1073,26 @@ int scsi_init_io(struct scsi_cmnd *cmd, GFP_ATOMIC);if (error) goto err_exit; + } + +if (blk_integrity_rq (cmd->request)) { +struct scsi_data_buffer *prot_sdb = cmd->prot_sdb; +int ivecs, count; + +BUG_ON (prot_sdb ==NULL ); + ivecs =blk_rq_count_integrity_sg (cmd->request); + +if (scsi_alloc_sgtable (prot_sdb, ivecs, gfp_mask)) { + error =BLKPREP_DEFER ; + goto err_exit; + } + + count =blk_rq_map_integrity_sg (cmd->request, + prot_sdb->table.sgl); +BUG_ON (unlikely (count > ivecs)); + + cmd->prot_sdb = prot_sdb; + cmd->prot_sdb->table.nents = count; }return BLKPREP_OK ;
drivers/scsi/scsi_priv.h +1 -0
@@ -77,6 +77,7 @@ struct request_queue;struct request_queue ;struct request ;extern int scsi_prep_fn (struct request_queue *,struct request *); +extern struct kmem_cache *scsi_sdb_cache;/* scsi_proc.c */ #ifdef CONFIG_SCSI_PROC_FS
include/scsi/scsi_cmnd.h +20 -0
@@ -90,6 +90,8 @@ struct scsi_cmnd {/* These elements define the operation we ultimately want to perform */ struct scsi_data_buffer sdb; +struct scsi_data_buffer *prot_sdb; +unsigned underflow;/* Return error if less than this amount is transferred */
@@ -274,4 +276,22 @@ static inline sector_t scsi_get_lba(stru return scmd->request->sector; } +static inline unsigned scsi_prot_sg_count (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.nents :0 ; +} + +static inline struct scatterlist *scsi_prot_sglist (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.sgl :NULL ; +} + +static inline struct scsi_data_buffer *scsi_prot (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb; +} + +#define scsi_for_each_prot_sg (cmd, sg, nseg, __i) \ + for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i) +#endif /* _SCSI_SCSI_CMND_H */
include/scsi/scsi_device.h +5 -0
@@ -416,6 +416,11 @@ static inline int scsi_device_enclosure(return sdev->inquiry[6 ] & (1 <<6 ); } +static inline int scsi_device_protection (struct scsi_device *sdev) +{ +return sdev->inquiry[5 ] & (1 <<0 ); +} +#define MODULE_ALIAS_SCSI_DEVICE (type) \ MODULE_ALIAS("scsi:t-" __stringify(type) "*")#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
Re: [PATCH 3 of 6] scsi: Support devices with protection information
In reply to Martin K. Petersendrivers/scsi/scsi.c scsi_pool_free_command(struct scsi_host_ · line 206
Quoted 12 lines · Martin K. Petersen
drivers/scsi/scsi.c +5
-0@@ -199,24 +199,60 @@ scsi_pool_free_command(struct scsi_host_ {kmem_cache_free (pool->sense_slab, cmd->sense_buffer);kmem_cache_free (pool->cmd_slab, cmd); + +if (cmd->prot_sdb) +kmem_cache_free (scsi_sdb_cache, cmd->prot_sdb); +} +
Shouldn't this check (and potential kmem_cache_free call) get done before freeing the cmd to the cmd_slab?
Mike
Re: [PATCH 3 of 6] scsi: Support devices with protection information
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Mike" == Michael Hernandez <[email protected]> writes:
Mike> Shouldn't this check (and potential kmem_cache_free call) get Mike> done before freeing the cmd to the cmd_slab?
*sigh* I think today is Brown Paper Bag Day for me.
But thanks - good spotting!
---8<---
scsi: Support devices with protection information
Implement support for DMA of protection information for devices that are data integrity capable.
- Add support for mapping an extra scatter-gather list containing the protection information.
- Allocate protection scsi_data_buffer if host is DIX (integrity DMA) capable.
- Accessor function for checking whether a device has protection enabled.
---
drivers/scsi/scsi.c +34 -2
@@ -197,8 +197,40 @@ scsi_pool_free_command(struct scsi_host_ scsi_pool_free_command(struct scsi_host_cmd_pool *pool, struct scsi_cmnd *cmd) { +if (cmd->prot_sdb) +kmem_cache_free (scsi_sdb_cache, cmd->prot_sdb); +kmem_cache_free (pool->sense_slab, cmd->sense_buffer);kmem_cache_free (pool->cmd_slab, cmd); +} + +/** +* scsi_host_alloc_command - internal function to allocate command +* @shost: SCSI host whose pool to allocate from +* @gfp_mask: mask for the allocation +* +* Returns a fully allocated command with sense buffer and protection +* data buffer (where applicable) or NULL on failure +*/ +static struct scsi_cmnd * +scsi_host_alloc_command (struct Scsi_Host *shost,gfp_t gfp_mask) +{ +struct scsi_cmnd *cmd; + + cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); +if (!cmd) +return NULL ; + +if (scsi_host_get_prot (shost) >=SHOST_DIX_TYPE0_PROTECTION ) { + cmd->prot_sdb =kmem_cache_zalloc (scsi_sdb_cache, gfp_mask); + +if (!cmd->prot_sdb) { +scsi_pool_free_command (shost->cmd_pool, cmd); +return NULL ; + } + } + +return cmd; }/**
@@ -214,7 +246,7 @@ struct scsi_cmnd *__scsi_get_command(strstruct scsi_cmnd *cmd;unsigned char *buf; - cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); + cmd =scsi_host_alloc_command (shost, gfp_mask);if (unlikely (!cmd)) {unsigned long flags;
@@ -457,7 +489,7 @@ int scsi_setup_command_freelist(struct S/* * Get one backup command for this host. */ - cmd =scsi_pool_alloc_command (shost->cmd_pool, gfp_mask); + cmd =scsi_host_alloc_command (shost, gfp_mask);if (!cmd) {scsi_put_host_cmd_pool (gfp_mask); shost->cmd_pool =NULL ;
drivers/scsi/scsi_lib.c +24 -1
@@ -65,7 +65,7 @@ static struct scsi_host_sg_pool scsi_sg_ };#undef SP -static struct kmem_cache *scsi_sdb_cache; +struct kmem_cache *scsi_sdb_cache;static void scsi_run_queue (struct request_queue *q);
@@ -787,6 +787,9 @@ void scsi_release_buffers(struct scsi_cmkmem_cache_free (scsi_sdb_cache, bidi_sdb); cmd->request->next_rq->special =NULL ; } + +if (scsi_prot_sg_count (cmd)) +scsi_free_sgtable (cmd->prot_sdb); }EXPORT_SYMBOL (scsi_release_buffers);
@@ -1070,6 +1073,26 @@ int scsi_init_io(struct scsi_cmnd *cmd, GFP_ATOMIC);if (error) goto err_exit; + } + +if (blk_integrity_rq (cmd->request)) { +struct scsi_data_buffer *prot_sdb = cmd->prot_sdb; +int ivecs, count; + +BUG_ON (prot_sdb ==NULL ); + ivecs =blk_rq_count_integrity_sg (cmd->request); + +if (scsi_alloc_sgtable (prot_sdb, ivecs, gfp_mask)) { + error =BLKPREP_DEFER ; + goto err_exit; + } + + count =blk_rq_map_integrity_sg (cmd->request, + prot_sdb->table.sgl); +BUG_ON (unlikely (count > ivecs)); + + cmd->prot_sdb = prot_sdb; + cmd->prot_sdb->table.nents = count; }return BLKPREP_OK ;
drivers/scsi/scsi_priv.h +1 -0
@@ -77,6 +77,7 @@ struct request_queue;struct request_queue ;struct request ;extern int scsi_prep_fn (struct request_queue *,struct request *); +extern struct kmem_cache *scsi_sdb_cache;/* scsi_proc.c */ #ifdef CONFIG_SCSI_PROC_FS
include/scsi/scsi_cmnd.h +20 -0
@@ -90,6 +90,8 @@ struct scsi_cmnd {/* These elements define the operation we ultimately want to perform */ struct scsi_data_buffer sdb; +struct scsi_data_buffer *prot_sdb; +unsigned underflow;/* Return error if less than this amount is transferred */
@@ -274,4 +276,22 @@ static inline sector_t scsi_get_lba(stru return scmd->request->sector; } +static inline unsigned scsi_prot_sg_count (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.nents :0 ; +} + +static inline struct scatterlist *scsi_prot_sglist (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb ? cmd->prot_sdb->table.sgl :NULL ; +} + +static inline struct scsi_data_buffer *scsi_prot (struct scsi_cmnd *cmd) +{ +return cmd->prot_sdb; +} + +#define scsi_for_each_prot_sg (cmd, sg, nseg, __i) \ + for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i) +#endif /* _SCSI_SCSI_CMND_H */
include/scsi/scsi_device.h +5 -0
@@ -416,6 +416,11 @@ static inline int scsi_device_enclosure(return sdev->inquiry[6 ] & (1 <<6 ); } +static inline int scsi_device_protection (struct scsi_device *sdev) +{ +return sdev->inquiry[5 ] & (1 <<0 ); +} +#define MODULE_ALIAS_SCSI_DEVICE (type) \ MODULE_ALIAS("scsi:t-" __stringify(type) "*")#define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
PATCH 1/1 sd: Support for SCSI disk (SBC) Data Integrity Field
In reply to Martin K. PetersenSupport for controllers and disks that implement DIF protection information:
- During command preparation the RDPROTECT/WRPROTECT must be set correctly if the target has DIF enabled.
- READ(6) and WRITE(6) are not supported when DIF is on.
- The controller must be told how to handle the I/O via the protection operation field in scsi_cmnd.
- Refactor the I/O completion code that extracts failed LBA from the returned sense data and handle DIF failures correctly.
- sd_dif.c implements the functions required to prepare and complete requests with protection information attached.
---
5 files changed, 647 insertions(+), 40 deletions(-) drivers/scsi/Kconfig | 1 drivers/scsi/Makefile | 2 drivers/scsi/sd.c | 121 +++++++---- drivers/scsi/sd.h | 25 ++ drivers/scsi/sd_dif.c | 538 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/Kconfig +1 -0
@@ -63,6 +63,7 @@ config BLK_DEV_SD config BLK_DEV_SD tristate "SCSI disk support" depends on SCSI + select CRC_T10DIF ---help--- If you want to use SCSI hard disks, Fibre Channel disks, Serial ATA (SATA) or Parallel ATA (PATA) hard disks,
drivers/scsi/Makefile +2 -0
@@ -151,6 +151,8 @@ scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_ scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o sd_mod-objs := sd.o +sd_mod-$(CONFIG_BLK_DEV_INTEGRITY) += sd_dif.o + sr_mod-objs := sr.o sr_ioctl.o sr_vendor.o ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \ := -DCONFIG_NCR53C8XX_PREFETCH -DSCSI_NCR_BIG_ENDIAN \
drivers/scsi/sd.c +81 -40
198 lines not on this page · raw message
drivers/scsi/sd.h +25 -0
@@ -82,4 +82,29 @@ enum sd_dif_target_protection_types { SD_DIF_TYPE3_PROTECTION = 0x3, }; +/* +* Data Integrity Field tuple. +*/ +struct sd_dif_tuple { +__be16 guard_tag;/* Checksum */ +__be16 app_tag;/* Opaque storage */ +__be32 ref_tag;/* Target LBA or indirect LBA */ +}; + +#if defined(CONFIG_BLK_DEV_INTEGRITY ) + +extern void sd_dif_op (struct scsi_cmnd *,unsigned int ,unsigned int ); +extern void sd_dif_config_host (struct scsi_disk *); +extern int sd_dif_prepare (struct request *rq,sector_t ,unsigned int ); +extern void sd_dif_complete (struct scsi_cmnd *,unsigned int ); + +#else /* CONFIG_BLK_DEV_INTEGRITY */ + +#define sd_dif_op (a, b, c) do { } while (0) +#define sd_dif_config_host (a) do { } while (0) +#define sd_dif_prepare (a, b, c) (0) +#define sd_dif_complete (a, b) (0) + +#endif /* CONFIG_BLK_DEV_INTEGRITY */ +#endif /* _SCSI_DISK_H */
drivers/scsi/sd_dif.c new file +538 -0
538 lines not on this page · raw message