Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
On Fri, Jun 02, 2017 at 03:01:29PM +0200, walter harms wrote: > > > Am 02.06.2017 14:39, schrieb Milan P. Gandhi: > > Simplify the check for return code of fcoe_if_init routine > > in fcoe_init function such that we could eliminate need for > > extra 'out_free' label and duplicate mutex_unlock statement. > > > > Signed-off-by: Milan P. Gandhi> > --- > > drivers/scsi/fcoe/fcoe.c | 7 +++ > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > > index ea21e7b..a2cf3d0 100644 > > --- a/drivers/scsi/fcoe/fcoe.c > > +++ b/drivers/scsi/fcoe/fcoe.c > > @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void) > > fcoe_dev_setup(); > > > > rc = fcoe_if_init(); > > + mutex_unlock(_config_mutex); > > + > > if (rc) > > - goto out_free; > > + goto out_destroy; > > > > - mutex_unlock(_config_mutex); > > return 0; > > > if you do that, why not > if (!rc) return 0; Gar... No. Please don't get creative with the last if statement. regards, dan carpenter
Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
I'm fine with this version... regards, dan carpenter
Re: [PATCH] Remove an extra out label in _fcoe_create function
On Thu, Jun 01, 2017 at 05:38:55PM +0530, Milan P. Gandhi wrote: > This patch removes an extra out label in _fcoe_create function > where we return if creation of FCOE interface is failed. > > Signed-off-by: Milan P. Gandhi> --- > drivers/scsi/fcoe/fcoe.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > index 7b960d3..ea21e7b 100644 > --- a/drivers/scsi/fcoe/fcoe.c > +++ b/drivers/scsi/fcoe/fcoe.c > @@ -2258,7 +2258,7 @@ static int _fcoe_create(struct net_device *netdev, enum > fip_mode fip_mode, > fcoe_interface_cleanup(fcoe); > mutex_unlock(_config_mutex); > fcoe_ctlr_device_delete(ctlr_dev); > - goto out; > + return rc; I don't like do nothing gotos, but I also don't like churn too much. It doesn't really make sense to do: rc = -EIO; ... return rc; We should probably preserve the error code? There is a second "return rc;" later which is more annoying. drivers/scsi/fcoe/fcoe.c 2274 /* 2275 * If the fcoe_ctlr_device is to be set to DISABLED 2276 * it must be done after the lport is added to the 2277 * hostlist, but before the rtnl_lock is released. 2278 * This is because the rtnl_lock protects the 2279 * hostlist that fcoe_device_notification uses. If 2280 * the FCoE Controller is intended to be created 2281 * DISABLED then 'enabled' needs to be considered 2282 * handling link events. 'enabled' must be set 2283 * before the lport can be found in the hostlist 2284 * when a link up event is received. 2285 */ 2286 if (link_state == FCOE_CREATE_LINK_UP) 2287 ctlr_dev->enabled = FCOE_CTLR_ENABLED; 2288 else 2289 ctlr_dev->enabled = FCOE_CTLR_DISABLED; 2290 2291 if (link_state == FCOE_CREATE_LINK_UP && 2292 !fcoe_link_ok(lport)) { 2293 rtnl_unlock(); 2294 fcoe_ctlr_link_up(ctlr); 2295 mutex_unlock(_config_mutex); 2296 return rc; ^ This is the same as "return 0;" and I guess it's supposed to be a success return? But it would look more clear if we changed it to return a literal instead of rc. 2297 } 2298 2299 out_nodev: 2300 rtnl_unlock(); regards, dan carpenter
Re: [PATCH-v2] target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout
Nicholas, > The people who are actively using iblock_execute_write_same_direct() > are doing so in the context of ESX VAAI BlockZero, together with > EXTENDED_COPY and COMPARE_AND_WRITE primitives. > > In practice though I've not seen any users of IBLOCK WRITE_SAME for > anything other than VAAI BlockZero, so just using > blkdev_issue_zeroout() when available, and falling back to > iblock_execute_write_same() if the WRITE_SAME buffer contains anything > other than zeros should be OK. > > (Hook up max_write_zeroes_sectors to signal LBPRZ feature bit in > target_configure_unmap_from_queue - nab) Looks reasonable. Reviewed-by: Martin K. Petersen-- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH V2 00/15] qedf: Update driver to version 8.18.22.0.
Chad, > Please apply the following patches to the scsi tree at your earliest > convenience. Applied to 4.13/scsi-queue, thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3 00/15] qla2xxx: Cleanup and minor fixes
Himanshu, > This series contains patches that were dropped from 4.12.0-rc3 > inclusion, since they can go to 4.13 merge window. Applied to 4.13/scsi-queue. Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v3 00/15] qla2xxx: Cleanup and minor fixes
On Fri, 2017-06-02 at 09:11 -0700, Himanshu Madhani wrote: > This series contains patches that were dropped from 4.12.0-rc3 inclusion, > since > they can go to 4.13 merge window. > > Changes from v2 --> v3 > o Added Reviewed-by tag from Bart. > o Droped couple patches for rework. > o Addressed minor comments from Bart where applicable. > > Changes from v1 --> v2 > o Addressed 0-day kernel warning. > o Addressed cleanups and updates as per Bart's comments. > o Added Acked-by tag from Nicholas to applicable patches. > > Please apply this series to for-next branch to be included in 4.13 merge > window. Hello Martin and Himanshu, All comments I had posted on this patch series have been addressed so I'm fine with this patch series. Bart.
Re: [PATCH] qla2xxx: remove writeq/readq function definitions
On Fri, 2017-06-02 at 13:32 +0200, Corentin Labbe wrote: > Instead of rewriting write/readq, use linux/io-64-nonatomic-lo-hi.h which > already have them. Reviewed-by: Bart Van Assche
Re: [PATCH v3 3/9] blk-mq: use the introduced blk_mq_unquiesce_queue()
On Fri, 2017-06-02 at 10:00 +0800, Ming Lei wrote: > On Thu, Jun 01, 2017 at 11:09:00PM +, Bart Van Assche wrote: > > My opinion is that scsi_internal_device_block() and > > scsi_internal_device_unblock() > > should be changed as follows for the scsi-mq code path: > > * scsi_internal_device_block() should call blk_mq_quiesce_queue(q) if the > > "wait" > > argument is true and should set the QUEUE_FLAG_QUIESCED flag if the "wait" > > argument is false. > > * scsi_internal_device_unblock() should call blk_mq_unquiesce_queue(). > > > > I am aware it sounds weird to set the QUEUE_FLAG_QUIESCED flag without > > waiting > > until ongoing .queue_rq() calls have finished. The only driver that triggers > > This way may break the contract of blk_mq_quiesce_queue() and is really weird. > And the above change shouldn't be related with this patchset, so better > to do whatever you want once this patch is merged. No, what I proposed doesn't break the contract of blk_mq_quiesce_queue(). The contract of that function is that if it is called that all pending .queue_rq() calls have finished and no new .queue_rq() calls will occur until blk_mq_unquiesce_queue() is called. Since the wait == false path does not call blk_mq_quiesce_queue() that contract does not apply. > This patchset won't break current SCSI uses of blk_mq_quiesce_queue() and > won't cause regression, and I setup one iSCSI target yesterday and it works > just fine, how about just keeping this patch as it is? As I explained in a previous e-mail, the iSCSI initiator driver does not trigger the wait == false path so your test did not trigger that code path. Something else I explained in a previous e-mail is that your patch makes it impossible to use the STOPPED flag in the SCSI core what it was originally intended for, namely preventing that the block layer keeps trying to queue requests while the block driver knows that it will have to return "BUSY". This is why I asked you to modify the SCSI integration of your patch. Bart.
Re: [PATCH] scsi: csiostor: add check for supported fw version
Varun, > T6 FCoE support is added in fw version 1.16.45.0 so return error if fw > version < 1.16.45.0 for T6 adapters. Applied to 4.13/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
[PATCH v3 04/12] Protect SCSI device state changes with a mutex
Serializing SCSI device state changes avoids that two state changes can occur concurrently, e.g. the state changes in scsi_target_block() and __scsi_remove_device(). This serialization is essential to make patch "Make __scsi_remove_device go straight from BLOCKED to DEL" work reliably. Enable this mechanism for all scsi_target_*block() callers but not for the scsi_internal_device_unblock() calls from the mpt3sas driver because that driver can call scsi_internal_device_unblock() from atomic context. Signed-off-by: Bart Van AsscheCc: Christoph Hellwig Cc: Hannes Reinecke Cc: Johannes Thumshirn --- drivers/scsi/scsi_error.c | 8 +++- drivers/scsi/scsi_lib.c | 27 +-- drivers/scsi/scsi_scan.c | 16 +--- drivers/scsi/scsi_sysfs.c | 24 +++- drivers/scsi/scsi_transport_srp.c | 7 --- drivers/scsi/sd.c | 7 +-- include/scsi/scsi_device.h| 1 + 7 files changed, 66 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index ecc07dab893d..ac3196420435 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1628,11 +1628,17 @@ static void scsi_eh_offline_sdevs(struct list_head *work_q, struct list_head *done_q) { struct scsi_cmnd *scmd, *next; + struct scsi_device *sdev; list_for_each_entry_safe(scmd, next, work_q, eh_entry) { sdev_printk(KERN_INFO, scmd->device, "Device offlined - " "not ready after error recovery\n"); - scsi_device_set_state(scmd->device, SDEV_OFFLINE); + sdev = scmd->device; + + mutex_lock(>state_mutex); + scsi_device_set_state(sdev, SDEV_OFFLINE); + mutex_unlock(>state_mutex); + scsi_eh_finish_cmd(scmd, done_q); } return; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index aa84b77e41dc..845d47244e70 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2881,7 +2881,12 @@ static void scsi_wait_for_queuecommand(struct scsi_device *sdev) int scsi_device_quiesce(struct scsi_device *sdev) { - int err = scsi_device_set_state(sdev, SDEV_QUIESCE); + int err; + + mutex_lock(>state_mutex); + err = scsi_device_set_state(sdev, SDEV_QUIESCE); + mutex_unlock(>state_mutex); + if (err) return err; @@ -2909,10 +2914,11 @@ void scsi_device_resume(struct scsi_device *sdev) * so assume the state is being managed elsewhere (for example * device deleted during suspend) */ - if (sdev->sdev_state != SDEV_QUIESCE || - scsi_device_set_state(sdev, SDEV_RUNNING)) - return; - scsi_run_queue(sdev->request_queue); + mutex_lock(>state_mutex); + if (sdev->sdev_state == SDEV_QUIESCE && + scsi_device_set_state(sdev, SDEV_RUNNING) == 0) + scsi_run_queue(sdev->request_queue); + mutex_unlock(>state_mutex); } EXPORT_SYMBOL(scsi_device_resume); @@ -3011,6 +3017,7 @@ static int scsi_internal_device_block(struct scsi_device *sdev) struct request_queue *q = sdev->request_queue; int err; + mutex_lock(>state_mutex); err = scsi_internal_device_block_nowait(sdev); if (err == 0) { if (q->mq_ops) @@ -3018,6 +3025,8 @@ static int scsi_internal_device_block(struct scsi_device *sdev) else scsi_wait_for_queuecommand(sdev); } + mutex_unlock(>state_mutex); + return err; } @@ -3088,7 +3097,13 @@ EXPORT_SYMBOL_GPL(scsi_internal_device_unblock_nowait); static int scsi_internal_device_unblock(struct scsi_device *sdev, enum scsi_device_state new_state) { - return scsi_internal_device_unblock_nowait(sdev, new_state); + int ret; + + mutex_lock(>state_mutex); + ret = scsi_internal_device_unblock_nowait(sdev, new_state); + mutex_unlock(>state_mutex); + + return ret; } static void diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 6f7128f49c30..e6de4eee97a3 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -231,6 +231,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, sdev->id = starget->id; sdev->lun = lun; sdev->channel = starget->channel; + mutex_init(>state_mutex); sdev->sdev_state = SDEV_CREATED; INIT_LIST_HEAD(>siblings); INIT_LIST_HEAD(>same_target_siblings); @@ -943,16 +944,17 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result, /* set the device running here so that slave configure * may do I/O */ +
Re: [PATCH] Remove an extra out label in _fcoe_create function
Milan, > This patch removes an extra out label in _fcoe_create function > where we return if creation of FCOE interface is failed. Applied to 4.13/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] Fix few small typos in fcoe.c
Milan, > This patch does a cleanup and fixes few small typos in fcoe.c Applied to 4.13/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
[PATCH v3 09/12] Make scsi_mq_prep_fn() call scsi_init_command()
This patch reduces code duplication. There are two functional changes in this patch: - It causes scsi_mq_prep_fn() to clear driver-private command data, just like the already upstream commit 1bad6c4a57ef ("scsi: zero per-cmd private driver data for each MQ I/O"). - The initialization of .prot_sdb is moved from scsi_mq_prep_fn() into scsi_init_request(). Signed-off-by: Bart Van AsscheCc: Hannes Reinecke Cc: Christoph Hellwig Cc: Johannes Thumshirn --- drivers/scsi/scsi_lib.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6b4fb48033fb..ef4e33319407 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1870,36 +1870,21 @@ static int scsi_mq_prep_fn(struct request *req) struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req); struct scsi_device *sdev = req->q->queuedata; struct Scsi_Host *shost = sdev->host; - unsigned char *sense_buf = cmd->sense_buffer; - unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; struct scatterlist *sg; - /* zero out the cmd, except for the embedded scsi_request */ - memset((char *)cmd + sizeof(cmd->req), 0, - sizeof(*cmd) - sizeof(cmd->req)); + scsi_init_command(sdev, cmd); req->special = cmd; cmd->request = req; - cmd->device = sdev; - cmd->sense_buffer = sense_buf; - cmd->flags = unchecked_isa_dma; cmd->tag = req->tag; - cmd->prot_op = SCSI_PROT_NORMAL; - INIT_LIST_HEAD(>list); - INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler); - cmd->jiffies_at_alloc = jiffies; - - scsi_add_cmd_to_list(cmd); - sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size; cmd->sdb.table.sgl = sg; if (scsi_host_get_prot(shost)) { - cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost); memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer)); cmd->prot_sdb->table.sgl = @@ -2025,6 +2010,7 @@ static int scsi_init_request(struct blk_mq_tag_set *set, struct request *rq, struct Scsi_Host *shost = set->driver_data; const bool unchecked_isa_dma = shost->unchecked_isa_dma; struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); + struct scatterlist *sg; if (unchecked_isa_dma) cmd->flags |= SCMD_UNCHECKED_ISA_DMA; @@ -2033,6 +2019,13 @@ static int scsi_init_request(struct blk_mq_tag_set *set, struct request *rq, if (!cmd->sense_buffer) return -ENOMEM; cmd->req.sense = cmd->sense_buffer; + + if (scsi_host_get_prot(shost)) { + sg = (void *)cmd + sizeof(struct scsi_cmnd) + + shost->hostt->cmd_size; + cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost); + } + return 0; } -- 2.12.2
[PATCH v3 07/12] Only add commands to the device command list if required by the LLD
Just like for the scsi-mq code path, in the single queue SCSI code path only add commands to the per-device command list if required by the SCSI LLD. This patch will make it easier to merge the single-queue and multiqueue command initialization code. Signed-off-by: Bart Van AsscheReviewed-by: Christoph Hellwig Reviewed-by: Hannes Reinecke Cc: Johannes Thumshirn --- drivers/scsi/scsi.c | 9 + drivers/scsi/scsi_lib.c | 52 +--- drivers/scsi/scsi_priv.h | 2 ++ 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 7bfbcfa7af40..485684aafb9b 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -108,14 +108,7 @@ EXPORT_SYMBOL(scsi_sd_pm_domain); */ void scsi_put_command(struct scsi_cmnd *cmd) { - unsigned long flags; - - /* serious error if the command hasn't come from a device list */ - spin_lock_irqsave(>device->list_lock, flags); - BUG_ON(list_empty(>list)); - list_del_init(>list); - spin_unlock_irqrestore(>device->list_lock, flags); - + scsi_del_cmd_from_list(cmd); BUG_ON(delayed_work_pending(>abort_work)); } diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 8665eccd2fc8..2c43b500e9f4 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -583,19 +583,9 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd) static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd) { - struct scsi_device *sdev = cmd->device; - struct Scsi_Host *shost = sdev->host; - unsigned long flags; - scsi_mq_free_sgtables(cmd); scsi_uninit_cmd(cmd); - - if (shost->use_cmd_list) { - BUG_ON(list_empty(>list)); - spin_lock_irqsave(>list_lock, flags); - list_del_init(>list); - spin_unlock_irqrestore(>list_lock, flags); - } + scsi_del_cmd_from_list(cmd); } /* @@ -1133,12 +1123,40 @@ int scsi_init_io(struct scsi_cmnd *cmd) } EXPORT_SYMBOL(scsi_init_io); +/* Add a command to the list used by the aacraid and dpt_i2o drivers */ +void scsi_add_cmd_to_list(struct scsi_cmnd *cmd) +{ + struct scsi_device *sdev = cmd->device; + struct Scsi_Host *shost = sdev->host; + unsigned long flags; + + if (shost->use_cmd_list) { + spin_lock_irqsave(>list_lock, flags); + list_add_tail(>list, >cmd_list); + spin_unlock_irqrestore(>list_lock, flags); + } +} + +/* Remove a command from the list used by the aacraid and dpt_i2o drivers */ +void scsi_del_cmd_from_list(struct scsi_cmnd *cmd) +{ + struct scsi_device *sdev = cmd->device; + struct Scsi_Host *shost = sdev->host; + unsigned long flags; + + if (shost->use_cmd_list) { + spin_lock_irqsave(>list_lock, flags); + BUG_ON(list_empty(>list)); + list_del_init(>list); + spin_unlock_irqrestore(>list_lock, flags); + } +} + void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) { void *buf = cmd->sense_buffer; void *prot = cmd->prot_sdb; unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; - unsigned long flags; /* zero out the cmd, except for the embedded scsi_request */ memset((char *)cmd + sizeof(cmd->req), 0, @@ -1151,9 +1169,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler); cmd->jiffies_at_alloc = jiffies; - spin_lock_irqsave(>list_lock, flags); - list_add_tail(>list, >cmd_list); - spin_unlock_irqrestore(>list_lock, flags); + scsi_add_cmd_to_list(cmd); } static int scsi_setup_scsi_cmnd(struct scsi_device *sdev, struct request *req) @@ -1870,11 +1886,7 @@ static int scsi_mq_prep_fn(struct request *req) INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler); cmd->jiffies_at_alloc = jiffies; - if (shost->use_cmd_list) { - spin_lock_irq(>list_lock); - list_add_tail(>list, >cmd_list); - spin_unlock_irq(>list_lock); - } + scsi_add_cmd_to_list(cmd); sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size; cmd->sdb.table.sgl = sg; diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h index f86057842f9a..c11c1f9c912c 100644 --- a/drivers/scsi/scsi_priv.h +++ b/drivers/scsi/scsi_priv.h @@ -80,6 +80,8 @@ int scsi_eh_get_sense(struct list_head *work_q, int scsi_noretry_cmd(struct scsi_cmnd *scmd); /* scsi_lib.c */ +extern void scsi_add_cmd_to_list(struct scsi_cmnd *cmd); +extern void scsi_del_cmd_from_list(struct scsi_cmnd *cmd); extern int scsi_maybe_unblock_host(struct scsi_device *sdev); extern void scsi_device_unbusy(struct
[PATCH v3 08/12] Introduce scsi_mq_sgl_size()
This patch does not change any functionality but makes the next patch easier to read. Signed-off-by: Bart Van AsscheReviewed-by: Christoph Hellwig Cc: Hannes Reinecke Cc: Johannes Thumshirn --- drivers/scsi/scsi_lib.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 2c43b500e9f4..6b4fb48033fb 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1858,6 +1858,13 @@ static inline int prep_to_mq(int ret) } } +/* Size in bytes of the sg-list stored in the scsi-mq command-private data. */ +static unsigned int scsi_mq_sgl_size(struct Scsi_Host *shost) +{ + return min_t(unsigned int, shost->sg_tablesize, SG_CHUNK_SIZE) * + sizeof(struct scatterlist); +} + static int scsi_mq_prep_fn(struct request *req) { struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req); @@ -1892,10 +1899,7 @@ static int scsi_mq_prep_fn(struct request *req) cmd->sdb.table.sgl = sg; if (scsi_host_get_prot(shost)) { - cmd->prot_sdb = (void *)sg + - min_t(unsigned int, - shost->sg_tablesize, SG_CHUNK_SIZE) * - sizeof(struct scatterlist); + cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost); memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer)); cmd->prot_sdb->table.sgl = @@ -2201,12 +2205,9 @@ struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev) int scsi_mq_setup_tags(struct Scsi_Host *shost) { - unsigned int cmd_size, sgl_size, tbl_size; + unsigned int cmd_size, sgl_size; - tbl_size = shost->sg_tablesize; - if (tbl_size > SG_CHUNK_SIZE) - tbl_size = SG_CHUNK_SIZE; - sgl_size = tbl_size * sizeof(struct scatterlist); + sgl_size = scsi_mq_sgl_size(shost); cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size; if (scsi_host_get_prot(shost)) cmd_size += sizeof(struct scsi_data_buffer) + sgl_size; -- 2.12.2
[PATCH v3 12/12] xen/scsifront: Remove code that zeroes driver-private command data
Since the SCSI core zeroes driver-private command data, remove that code from the xen-scsifront driver. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Juergen Gross Reviewed-by: Christoph Hellwig Cc: xen-de...@lists.xenproject.org Cc: Johannes Thumshirn --- drivers/scsi/xen-scsifront.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index a6a8b60d4902..36f59a1be7e9 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -534,7 +534,6 @@ static int scsifront_queuecommand(struct Scsi_Host *shost, int err; sc->result = 0; - memset(shadow, 0, sizeof(*shadow)); shadow->sc = sc; shadow->act = VSCSIIF_ACT_SCSI_CDB; -- 2.12.2
[PATCH v3 10/12] snic: Remove code that zeroes driver-private command data
Since the SCSI core zeroes driver-private command data, remove that code from the snic driver. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Narsimhulu Musini Reviewed-by: Christoph Hellwig Cc: Sesidhar Baddela Cc: Johannes Thumshirn --- drivers/scsi/snic/snic_scsi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c index da979a73baa0..05c3a7282d4a 100644 --- a/drivers/scsi/snic/snic_scsi.c +++ b/drivers/scsi/snic/snic_scsi.c @@ -359,8 +359,6 @@ snic_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *sc) SNIC_SCSI_DBG(shost, "sc %p Tag %d (sc %0x) lun %lld in snic_qcmd\n", sc, snic_cmd_tag(sc), sc->cmnd[0], sc->device->lun); - memset(scsi_cmd_priv(sc), 0, sizeof(struct snic_internal_io_state)); - ret = snic_issue_scsi_req(snic, tgt, sc); if (ret) { SNIC_HOST_ERR(shost, "Failed to Q, Scsi Req w/ err %d.\n", ret); -- 2.12.2
[PATCH v3 01/12] Avoid that scsi_exit_rq() triggers a use-after-free
Dereferencing shost from scsi_exit_rq() is not safe because the SCSI host may already have been freed when scsi_exit_rq() is called. Increasing the shost reference count in scsi_init_rq() and dropping that reference in scsi_exit_rq() is nontrivial since scsi_host_dev_release() may sleep and since scsi_exit_rq() may be called from interrupt context. Since scsi_exit_rq() only needs a single bit from shost, copy that bit into struct scsi_cmnd. Reported-by: Scott BauerFixes: e9c787e65c0c ("scsi: allocate scsi_cmnd structures as part of struct request") Signed-off-by: Bart Van Assche Reviewed-by: Christoph Hellwig Cc: Hannes Reinecke Cc: Scott Bauer Cc: Jan Kara Cc: --- drivers/scsi/scsi_lib.c | 47 +-- include/scsi/scsi_cmnd.h | 1 + 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 814a4bd8405d..cc9f792cd12b 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -44,23 +44,23 @@ static struct kmem_cache *scsi_sense_isadma_cache; static DEFINE_MUTEX(scsi_sense_cache_mutex); static inline struct kmem_cache * -scsi_select_sense_cache(struct Scsi_Host *shost) +scsi_select_sense_cache(bool unchecked_isa_dma) { - return shost->unchecked_isa_dma ? - scsi_sense_isadma_cache : scsi_sense_cache; + return unchecked_isa_dma ? scsi_sense_isadma_cache : scsi_sense_cache; } -static void scsi_free_sense_buffer(struct Scsi_Host *shost, - unsigned char *sense_buffer) +static void scsi_free_sense_buffer(bool unchecked_isa_dma, + unsigned char *sense_buffer) { - kmem_cache_free(scsi_select_sense_cache(shost), sense_buffer); + kmem_cache_free(scsi_select_sense_cache(unchecked_isa_dma), + sense_buffer); } -static unsigned char *scsi_alloc_sense_buffer(struct Scsi_Host *shost, +static unsigned char *scsi_alloc_sense_buffer(bool unchecked_isa_dma, gfp_t gfp_mask, int numa_node) { - return kmem_cache_alloc_node(scsi_select_sense_cache(shost), gfp_mask, - numa_node); + return kmem_cache_alloc_node(scsi_select_sense_cache(unchecked_isa_dma), +gfp_mask, numa_node); } int scsi_init_sense_cache(struct Scsi_Host *shost) @@ -68,7 +68,7 @@ int scsi_init_sense_cache(struct Scsi_Host *shost) struct kmem_cache *cache; int ret = 0; - cache = scsi_select_sense_cache(shost); + cache = scsi_select_sense_cache(shost->unchecked_isa_dma); if (cache) return 0; @@ -1137,6 +1137,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) { void *buf = cmd->sense_buffer; void *prot = cmd->prot_sdb; + unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; unsigned long flags; /* zero out the cmd, except for the embedded scsi_request */ @@ -1146,6 +1147,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) cmd->device = dev; cmd->sense_buffer = buf; cmd->prot_sdb = prot; + cmd->flags = unchecked_isa_dma; INIT_DELAYED_WORK(>abort_work, scmd_eh_abort_handler); cmd->jiffies_at_alloc = jiffies; @@ -1846,6 +1848,7 @@ static int scsi_mq_prep_fn(struct request *req) struct scsi_device *sdev = req->q->queuedata; struct Scsi_Host *shost = sdev->host; unsigned char *sense_buf = cmd->sense_buffer; + unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; struct scatterlist *sg; /* zero out the cmd, except for the embedded scsi_request */ @@ -1857,6 +1860,7 @@ static int scsi_mq_prep_fn(struct request *req) cmd->request = req; cmd->device = sdev; cmd->sense_buffer = sense_buf; + cmd->flags = unchecked_isa_dma; cmd->tag = req->tag; @@ -2003,10 +2007,13 @@ static int scsi_init_request(struct blk_mq_tag_set *set, struct request *rq, unsigned int hctx_idx, unsigned int numa_node) { struct Scsi_Host *shost = set->driver_data; + const bool unchecked_isa_dma = shost->unchecked_isa_dma; struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); - cmd->sense_buffer = - scsi_alloc_sense_buffer(shost, GFP_KERNEL, numa_node); + if (unchecked_isa_dma) + cmd->flags |= SCMD_UNCHECKED_ISA_DMA; + cmd->sense_buffer = scsi_alloc_sense_buffer(unchecked_isa_dma, + GFP_KERNEL, numa_node); if (!cmd->sense_buffer) return -ENOMEM; cmd->req.sense = cmd->sense_buffer; @@ -2016,10 +2023,10 @@ static int scsi_init_request(struct blk_mq_tag_set *set, struct request
[PATCH v3 11/12] virtio_scsi: Remove code that zeroes driver-private command data
Since the SCSI core zeroes driver-private command data, remove that code from the virtio driver. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Cc: Michael S. Tsirkin Cc: Johannes Thumshirn --- drivers/scsi/virtio_scsi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index f8dbfeee6c63..dc2e97c543a5 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -547,7 +547,6 @@ static int virtscsi_queuecommand(struct virtio_scsi *vscsi, dev_dbg(>device->sdev_gendev, "cmd %p CDB: %#02x\n", sc, sc->cmnd[0]); - memset(cmd, 0, sizeof(*cmd)); cmd->sc = sc; BUG_ON(sc->cmd_len > VIRTIO_SCSI_CDB_SIZE); -- 2.12.2
[PATCH v3 06/12] Make __scsi_remove_device go straight from BLOCKED to DEL
If a device is blocked, make __scsi_remove_device() cause it to transition to the DEL state. This means that all the commands issued in .shutdown() will error in the mid-layer, thus making the removal proceed without being stopped. This patch is a slightly modified version of a patch from James Bottomley. This patch avoids that the following lockup occurs: Call Trace: schedule+0x35/0x80 schedule_timeout+0x237/0x2d0 io_schedule_timeout+0xa6/0x110 wait_for_completion_io+0xa3/0x110 blk_execute_rq+0xdf/0x120 scsi_execute+0xce/0x150 [scsi_mod] scsi_execute_req_flags+0x8f/0xf0 [scsi_mod] sd_sync_cache+0xa9/0x190 [sd_mod] sd_shutdown+0x6a/0x100 [sd_mod] sd_remove+0x64/0xc0 [sd_mod] __device_release_driver+0x8d/0x120 device_release_driver+0x1e/0x30 bus_remove_device+0xf9/0x170 device_del+0x127/0x240 __scsi_remove_device+0xc1/0xd0 [scsi_mod] scsi_forget_host+0x57/0x60 [scsi_mod] scsi_remove_host+0x72/0x110 [scsi_mod] srp_remove_work+0x8b/0x200 [ib_srp] Reported-by: Israel RukshinSigned-off-by: Bart Van Assche Reviewed-by: Hannes Reinecke Cc: James Bottomley Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Benjamin Block --- drivers/scsi/scsi_lib.c | 2 +- drivers/scsi/scsi_sysfs.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6a58a124714f..8665eccd2fc8 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2624,7 +2624,6 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) case SDEV_QUIESCE: case SDEV_OFFLINE: case SDEV_TRANSPORT_OFFLINE: - case SDEV_BLOCK: break; default: goto illegal; @@ -2638,6 +2637,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) case SDEV_OFFLINE: case SDEV_TRANSPORT_OFFLINE: case SDEV_CANCEL: + case SDEV_BLOCK: case SDEV_CREATED_BLOCK: break; default: diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index a91537a3abbf..ce470f62a8ae 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1290,7 +1290,17 @@ void __scsi_remove_device(struct scsi_device *sdev) * wait until it has finished before changing the device state. */ mutex_lock(>state_mutex); + /* +* If blocked, we go straight to DEL and restart the queue so +* any commands issued during driver shutdown (like sync +* cache) are errored immediately. +*/ res = scsi_device_set_state(sdev, SDEV_CANCEL); + if (res != 0) { + res = scsi_device_set_state(sdev, SDEV_DEL); + if (res == 0) + scsi_start_queue(sdev); + } mutex_unlock(>state_mutex); if (res != 0) -- 2.12.2
[PATCH v3 05/12] Introduce scsi_start_queue()
This patch does not change any functionality. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Christoph Hellwig Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Benjamin Block --- drivers/scsi/scsi_lib.c | 25 +++-- drivers/scsi/scsi_priv.h | 1 + 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 845d47244e70..6a58a124714f 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -3030,6 +3030,20 @@ static int scsi_internal_device_block(struct scsi_device *sdev) return err; } +void scsi_start_queue(struct scsi_device *sdev) +{ + struct request_queue *q = sdev->request_queue; + unsigned long flags; + + if (q->mq_ops) { + blk_mq_start_stopped_hw_queues(q, false); + } else { + spin_lock_irqsave(q->queue_lock, flags); + blk_start_queue(q); + spin_unlock_irqrestore(q->queue_lock, flags); + } +} + /** * scsi_internal_device_unblock_nowait - resume a device after a block request * @sdev: device to resume @@ -3048,9 +3062,6 @@ static int scsi_internal_device_block(struct scsi_device *sdev) int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, enum scsi_device_state new_state) { - struct request_queue *q = sdev->request_queue; - unsigned long flags; - /* * Try to transition the scsi device to SDEV_RUNNING or one of the * offlined states and goose the device queue if successful. @@ -3068,13 +3079,7 @@ int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, sdev->sdev_state != SDEV_OFFLINE) return -EINVAL; - if (q->mq_ops) { - blk_mq_start_stopped_hw_queues(q, false); - } else { - spin_lock_irqsave(q->queue_lock, flags); - blk_start_queue(q); - spin_unlock_irqrestore(q->queue_lock, flags); - } + scsi_start_queue(sdev); return 0; } diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h index 59ebc1795bb3..f86057842f9a 100644 --- a/drivers/scsi/scsi_priv.h +++ b/drivers/scsi/scsi_priv.h @@ -88,6 +88,7 @@ extern void scsi_run_host_queues(struct Scsi_Host *shost); extern void scsi_requeue_run_queue(struct work_struct *work); extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev); extern struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev); +extern void scsi_start_queue(struct scsi_device *sdev); extern int scsi_mq_setup_tags(struct Scsi_Host *shost); extern void scsi_mq_destroy_tags(struct Scsi_Host *shost); extern int scsi_init_queue(void); -- 2.12.2
[PATCH v3 02/12] Split scsi_internal_device_block()
Instead of passing a "wait" argument to scsi_internal_device_block(), split this function into a function that waits and a function that doesn't wait. This will make it easier to serialize SCSI device state changes through a mutex. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Johannes Thumshirn Reviewed-by: Christoph Hellwig Cc: Sreekanth Reddy --- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 4 +- drivers/scsi/scsi_lib.c | 73 +++- include/scsi/scsi_device.h | 2 +- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index a5d872664257..c63bc5ccce37 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -2859,7 +2859,7 @@ _scsih_internal_device_block(struct scsi_device *sdev, sas_device_priv_data->sas_target->handle); sas_device_priv_data->block = 1; - r = scsi_internal_device_block(sdev, false); + r = scsi_internal_device_block_nowait(sdev); if (r == -EINVAL) sdev_printk(KERN_WARNING, sdev, "device_block failed with return(%d) for handle(0x%04x)\n", @@ -2895,7 +2895,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev, "performing a block followed by an unblock\n", r, sas_device_priv_data->sas_target->handle); sas_device_priv_data->block = 1; - r = scsi_internal_device_block(sdev, false); + r = scsi_internal_device_block_nowait(sdev); if (r) sdev_printk(KERN_WARNING, sdev, "retried device_block " "failed with return(%d) for handle(0x%04x)\n", diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index cc9f792cd12b..c9ce36d16df0 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2943,28 +2943,20 @@ scsi_target_resume(struct scsi_target *starget) EXPORT_SYMBOL(scsi_target_resume); /** - * scsi_internal_device_block - internal function to put a device temporarily into the SDEV_BLOCK state - * @sdev: device to block - * @wait: Whether or not to wait until ongoing .queuecommand() / - * .queue_rq() calls have finished. + * scsi_internal_device_block_nowait - try to transition to the SDEV_BLOCK state + * @sdev: device to block * - * Block request made by scsi lld's to temporarily stop all - * scsi commands on the specified device. May sleep. + * Pause SCSI command processing on the specified device. Does not sleep. * - * Returns zero if successful or error if not + * Returns zero if successful or a negative error code upon failure. * - * Notes: - * This routine transitions the device to the SDEV_BLOCK state - * (which must be a legal transition). When the device is in this - * state, all commands are deferred until the scsi lld reenables - * the device with scsi_device_unblock or device_block_tmo fires. - * - * To do: avoid that scsi_send_eh_cmnd() calls queuecommand() after - * scsi_internal_device_block() has blocked a SCSI device and also - * remove the rport mutex lock and unlock calls from srp_queuecommand(). + * Notes: + * This routine transitions the device to the SDEV_BLOCK state (which must be + * a legal transition). When the device is in this state, command processing + * is paused until the device leaves the SDEV_BLOCK state. See also + * scsi_internal_device_unblock_nowait(). */ -int -scsi_internal_device_block(struct scsi_device *sdev, bool wait) +int scsi_internal_device_block_nowait(struct scsi_device *sdev) { struct request_queue *q = sdev->request_queue; unsigned long flags; @@ -2984,21 +2976,50 @@ scsi_internal_device_block(struct scsi_device *sdev, bool wait) * request queue. */ if (q->mq_ops) { - if (wait) - blk_mq_quiesce_queue(q); - else - blk_mq_stop_hw_queues(q); + blk_mq_stop_hw_queues(q); } else { spin_lock_irqsave(q->queue_lock, flags); blk_stop_queue(q); spin_unlock_irqrestore(q->queue_lock, flags); - if (wait) - scsi_wait_for_queuecommand(sdev); } return 0; } -EXPORT_SYMBOL_GPL(scsi_internal_device_block); +EXPORT_SYMBOL_GPL(scsi_internal_device_block_nowait); + +/** + * scsi_internal_device_block - try to transition to the SDEV_BLOCK state + * @sdev: device to block + * + * Pause SCSI command processing on the specified device and wait until all + * ongoing scsi_request_fn() / scsi_queue_rq() calls have finished. May sleep. + * + * Returns zero if successful or a negative error code upon failure. + * + *
[PATCH v3 03/12] Create two versions of scsi_internal_device_unblock()
This will make it easier to serialize SCSI device state changes through a mutex. Signed-off-by: Bart Van AsscheReviewed-by: Hannes Reinecke Reviewed-by: Johannes Thumshirn Reviewed-by: Christoph Hellwig Cc: Sreekanth Reddy --- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 4 ++-- drivers/scsi/scsi_lib.c | 46 +--- include/scsi/scsi_device.h | 4 ++-- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index c63bc5ccce37..22998cbd538f 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -2883,7 +2883,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev, sdev_printk(KERN_WARNING, sdev, "device_unblock and setting to running, " "handle(0x%04x)\n", sas_device_priv_data->sas_target->handle); sas_device_priv_data->block = 0; - r = scsi_internal_device_unblock(sdev, SDEV_RUNNING); + r = scsi_internal_device_unblock_nowait(sdev, SDEV_RUNNING); if (r == -EINVAL) { /* The device has been set to SDEV_RUNNING by SD layer during * device addition but the request queue is still stopped by @@ -2902,7 +2902,7 @@ _scsih_internal_device_unblock(struct scsi_device *sdev, r, sas_device_priv_data->sas_target->handle); sas_device_priv_data->block = 0; - r = scsi_internal_device_unblock(sdev, SDEV_RUNNING); + r = scsi_internal_device_unblock_nowait(sdev, SDEV_RUNNING); if (r) sdev_printk(KERN_WARNING, sdev, "retried device_unblock" " failed with return(%d) for handle(0x%04x)\n", diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index c9ce36d16df0..aa84b77e41dc 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -3022,24 +3022,22 @@ static int scsi_internal_device_block(struct scsi_device *sdev) } /** - * scsi_internal_device_unblock - resume a device after a block request + * scsi_internal_device_unblock_nowait - resume a device after a block request * @sdev: device to resume - * @new_state: state to set devices to after unblocking + * @new_state: state to set the device to after unblocking * - * Called by scsi lld's or the midlayer to restart the device queue - * for the previously suspended scsi device. Called from interrupt or - * normal process context. + * Restart the device queue for a previously suspended SCSI device. Does not + * sleep. * - * Returns zero if successful or error if not. + * Returns zero if successful or a negative error code upon failure. * - * Notes: - * This routine transitions the device to the SDEV_RUNNING state - * or to one of the offline states (which must be a legal transition) - * allowing the midlayer to goose the queue for this device. + * Notes: + * This routine transitions the device to the SDEV_RUNNING state or to one of + * the offline states (which must be a legal transition) allowing the midlayer + * to goose the queue for this device. */ -int -scsi_internal_device_unblock(struct scsi_device *sdev, -enum scsi_device_state new_state) +int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, + enum scsi_device_state new_state) { struct request_queue *q = sdev->request_queue; unsigned long flags; @@ -3071,7 +3069,27 @@ scsi_internal_device_unblock(struct scsi_device *sdev, return 0; } -EXPORT_SYMBOL_GPL(scsi_internal_device_unblock); +EXPORT_SYMBOL_GPL(scsi_internal_device_unblock_nowait); + +/** + * scsi_internal_device_unblock - resume a device after a block request + * @sdev: device to resume + * @new_state: state to set the device to after unblocking + * + * Restart the device queue for a previously suspended SCSI device. May sleep. + * + * Returns zero if successful or a negative error code upon failure. + * + * Notes: + * This routine transitions the device to the SDEV_RUNNING state or to one of + * the offline states (which must be a legal transition) allowing the midlayer + * to goose the queue for this device. + */ +static int scsi_internal_device_unblock(struct scsi_device *sdev, + enum scsi_device_state new_state) +{ + return scsi_internal_device_unblock_nowait(sdev, new_state); +} static void device_block(struct scsi_device *sdev, void *data) diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 6ce6888f3c69..5f24dae2a8e1 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -473,8 +473,8 @@ static inline int scsi_device_created(struct scsi_device *sdev) } int
[PATCH v3 00/12] SCSI patches for kernel v4.13
Hello Martin, This patch series consists of the bug fixes and improvements I came up with during the past two months. This patch series has been developed on top of your 4.13/scsi-queue branch. Please consider these patches for kernel v4.13. Thanks, Bart. The changes compared to v2 of this patch series are: - Addressed Christoph's review comments: added an explanation to patch "Protect SCSI device state changes with a mutex" of why that change is needed. Removed a printk() from patch "Make __scsi_remove_device go straight from BLOCKED to DEL". For scsi-mq, moved the initialization of .prot_sdb from scsi_mq_prep_fn() into scsi_init_request(). Fixed the driver name in the virtio_scsi patch. Between v1 and v2: - Left out the block layer patches from this series. - Reworked this patch series such that it applies cleanly on the 4.13 SCSI patch queue and no longer depends on any block layer changes that are not yet upstream. - In patch "Avoid that scsi_exit_rq() triggers a use-after-free", make the prep functions save and restore the SCMD_UNCHECKED_ISA_DMA flag. - Added patch "Introduce scsi_start_queue()". Bart Van Assche (12): Avoid that scsi_exit_rq() triggers a use-after-free Split scsi_internal_device_block() Create two versions of scsi_internal_device_unblock() Protect SCSI device state changes with a mutex Introduce scsi_start_queue() Make __scsi_remove_device go straight from BLOCKED to DEL Only add commands to the device command list if required by the LLD Introduce scsi_mq_sgl_size() Make scsi_mq_prep_fn() call scsi_init_command() snic: Remove code that zeroes driver-private command data virtio_scsi: Remove code that zeroes driver-private command data xen/scsifront: Remove code that zeroes driver-private command data drivers/scsi/mpt3sas/mpt3sas_scsih.c | 8 +- drivers/scsi/scsi.c | 9 +- drivers/scsi/scsi_error.c| 8 +- drivers/scsi/scsi_lib.c | 306 ++- drivers/scsi/scsi_priv.h | 3 + drivers/scsi/scsi_scan.c | 16 +- drivers/scsi/scsi_sysfs.c| 34 +++- drivers/scsi/scsi_transport_srp.c| 7 +- drivers/scsi/sd.c| 7 +- drivers/scsi/snic/snic_scsi.c| 2 - drivers/scsi/virtio_scsi.c | 1 - drivers/scsi/xen-scsifront.c | 1 - include/scsi/scsi_cmnd.h | 1 + include/scsi/scsi_device.h | 7 +- 14 files changed, 258 insertions(+), 152 deletions(-) -- 2.12.2
Re: [PATCH v2 00/12] SCSI patches for kernel v4.13
On Fri, 2017-06-02 at 17:08 -0400, Martin K. Petersen wrote: > > This patch series consists of the bug fixes and improvements I came up > > with during the past two months. Please consider these patches for > > kernel v4.13. > > Does this series have dependencies on stuff that went into block for > 4.13? Hello Martin, This series does not have any dependencies on pending block layer changes and applies fine on your 4.13/queue branch. BTW, I just finished addressing the review comments posted on v2 and am ready to post v3 of this patch series. Bart.
Re: [PATCH v2 00/12] SCSI patches for kernel v4.13
Bart, > This patch series consists of the bug fixes and improvements I came up > with during the past two months. Please consider these patches for > kernel v4.13. Does this series have dependencies on stuff that went into block for 4.13? -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 11/12] virtio: Remove code that zeroes driver-private command data
On Fri, 2017-06-02 at 09:23 +0200, Christoph Hellwig wrote: > Nit: the driver name is virtio_scsi, not just virtio. Hello Christoph, I will update the patch title. Bart.
Re: [PATCH v2 06/12] Make __scsi_remove_device go straight from BLOCKED to DEL
On Fri, 2017-06-02 at 09:18 +0200, Christoph Hellwig wrote: > On Thu, Jun 01, 2017 at 04:27:05PM -0700, Bart Van Assche wrote: > > If a device is blocked, make __scsi_remove_device() cause it to > > transition to the DEL state. This means that all the commands > > issued in .shutdown() will error in the mid-layer, thus making > > the removal proceed without being stopped. > > > > This patch is a slightly modified version of a patch from James > > Bottomley. This patch avoids that the following lockup occurs: > > Do we really need the printk for this case? And if we do we should > probably rate limit it. Hello Christoph, The printk statement was useful during debugging to confirm that the code path with the printk was hit. I will remove the printk. Bart.
Re: SCSI controller with CDB-16
Alexander, > - Ultra320 bus does not work physically, too many errors. > - Ultra160 bus works relable, no errors. Ultra320 was generally problematic. I worked on two different hardware platforms that both gave up on the original plan of using 320 and went back to an Ultra160 ASIC between prototype and GA. Mainly due to signaling problems. Even internal controller to disk backplane cabling was a problem. So make sure your cabling and termination is in good working order and rated for 320. > Unfortunately Adaptec does not work with CDB-16 too :( I don't have any SPI hardware anymore. But my suggestion would be to look for an LSI 53C1030-based card. It's a different ASIC than the 1010 you tried (has more in common with the first gen SAS controllers). It's the final generation of SPI products and would stand the best chance of working. -- Martin K. Petersen Oracle Linux Engineering
[PATCH] qedf: Check if sense buffer has been allocated during completion.
NOTE: This should be applied after the series 'qedf: Update driver to version 8.18.22.0.' sc_cmd->sense_buffer is not guaranteed to be allocated so we need to check if the pointer is NULL before trying to copy anything into it. Fixes the crash: [ 143.793176] [:00:00.0]:[qedf_eh_device_reset:626]: LUN RESET Issued... [ 143.802996] BUG: unable to handle kernel NULL pointer dereference at (null) [ 143.803063] IP: qedf_parse_fcp_rsp+0xe2/0x290 [qedf] [ 143.803077] PGD 0 [ 143.803078] P4D 0 [ 143.803103] Oops: 0002 [#1] SMP [ 143.803115] Modules linked in: msr(E) ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) raw(E) scsi_transport_iscsi(E) br_netfilter(E) bridge(E) iscsi_ibft(E) iscsi_boot_sysfs(E) intel_rapl(E) sb_edac(E) x86_pkg_temp_thermal(E) intel_powerclamp(E) coretemp(E) kvm_intel(E) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) xfs(E) ghash_clmulni_intel(E) pcbc(E) aesni_intel(E) aes_x86_64(E) crypto_simd(E) ipmi_ssif(E) glue_helper(E) iTCO_wdt(E) iTCO_vendor_support(E) lpc_ich(E) ipmi_si(E) pcspkr(E) hpilo(E) ioatdma(E) cryptd(E) ipmi_devintf(E) hpwdt(E) mfd_core(E) shpchp(E) dca(E) thermal(E) pcc_cpufreq(E) ipmi_msghandler(E) acpi_cpufreq(E) af_packet(E) btrfs(E) xor(E) raid6_pq(E) sr_mod(E) cdrom(E) ata_generic(E) sd_mod(E) 8021q(E) garp(E) [ 143.803302] stp(E) llc(E) mrp(E) bnx2fc(E) cnic(E) uio(E) mgag200(E) ata_piix(E) i2c_algo_bit(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) ahci(E) fb_sys_fops(E) bnx2x(E) qedf(E) serio_raw(E) libahci(E) ttm(E) uhci_hcd(E) ehci_pci(E) qed(E) mdio(E) libcrc32c(E) ehci_hcd(E) crc32c_intel(E) drm(E) libata(E) usbcore(E) tg3(E) ptp(E) hpsa(E) pps_core(E) scsi_transport_sas(E) libphy(E) wmi(E) button(E) fcoe(E) libfcoe(E) libfc(E) scsi_transport_fc(E) sg(E) dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) scsi_mod(E) autofs4(E) [ 143.803438] CPU: 31 PID: 494 Comm: kworker/31:2 Tainted: GE 4.12.0-rc1-69-default+ #1 [ 143.803461] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 08/20/2012 [ 143.803480] Workqueue: qedf_io_wq qedf_fp_io_handler [qedf] [ 143.803496] task: 8804181a task.stack: c90003b64000 [ 143.803514] RIP: 0010:qedf_parse_fcp_rsp+0xe2/0x290 [qedf] [ 143.803529] RSP: 0018:c90003b67dc8 EFLAGS: 00010246 [ 143.803544] RAX: RBX: 880401abdd48 RCX: 000c [ 143.803563] RDX: 0060 RSI: a039c740 RDI: [ 143.803581] RBP: c90003b67df0 R08: a039dba8 R09: [ 143.803600] R10: R11: 0018 R12: [ 143.803619] R13: 88040ac80bc8 R14: 0008 R15: 880407c14008 [ 143.803638] FS: () GS:88043f7c() knlGS: [ 143.804360] CS: 0010 DS: ES: CR0: 80050033 [ 143.805065] CR2: CR3: 01c09000 CR4: 000406e0 [ 143.805753] Call Trace: [ 143.806436] qedf_process_tmf_compl+0x19/0x30 [qedf] [ 143.807124] qedf_process_cqe+0x265/0x280 [qedf] [ 143.807800] qedf_fp_io_handler+0x26/0x60 [qedf] [ 143.808469] process_one_work+0x138/0x370 [ 143.809133] worker_thread+0x4d/0x3b0 [ 143.809797] kthread+0x109/0x140 [ 143.810451] ? rescuer_thread+0x320/0x320 [ 143.811100] ? kthread_park+0x60/0x60 [ 143.811743] ret_from_fork+0x2c/0x40 Signed-off-by: Chad Dupuis--- drivers/scsi/qedf/qedf_io.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c index ea37c78..ded3860 100644 --- a/drivers/scsi/qedf/qedf_io.c +++ b/drivers/scsi/qedf/qedf_io.c @@ -1041,10 +1041,13 @@ static void qedf_parse_fcp_rsp(struct qedf_ioreq *io_req, fcp_sns_len = SCSI_SENSE_BUFFERSIZE; } - memset(sc_cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); - if (fcp_sns_len) - memcpy(sc_cmd->sense_buffer, sense_data, - fcp_sns_len); + /* The sense buffer can be NULL for TMF commands */ + if (sc_cmd->sense_buffer) { + memset(sc_cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); + if (fcp_sns_len) + memcpy(sc_cmd->sense_buffer, sense_data, + fcp_sns_len); + } } static void qedf_unmap_sg_list(struct qedf_ctx *qedf, struct qedf_ioreq *io_req) -- 1.8.5.6
Re: [PATCH] scsi: cxgb4i,libcxgbi: in error case RST tcp conn
Varun, > If logout response is not received and ->ep_disconnect() is called > then close tcp conn by RST instead of FIN to cleanup conn resources > immediately. > > Also move ->csk_push_tx_frames() above 'done:' to avoid calling > ->csk_push_tx_frames() in error cases. Applied to 4.12/scsi-fixes. Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 09/12] Make scsi_mq_prep_fn() call scsi_init_command()
On Fri, 2017-06-02 at 09:22 +0200, Christoph Hellwig wrote: > I like this idea, but.. > > > -void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) > > +void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd, > > + struct scsi_data_buffer *prot_sdb) > > { > > void *buf = cmd->sense_buffer; > > - void *prot = cmd->prot_sdb; > > unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; > > > > /* zero out the cmd, except for the embedded scsi_request */ > > @@ -1164,7 +1164,7 @@ void scsi_init_command(struct scsi_device *dev, > > struct scsi_cmnd *cmd) > > > > cmd->device = dev; > > cmd->sense_buffer = buf; > > - cmd->prot_sdb = prot; > > + cmd->prot_sdb = prot_sdb; > > What would be the problem of always preserving the original prot_sdb > value instead of passing it by argument? You;d just need to initialize > it in scsi_init_request when the command is allocated. Hello Christoph, That sounds like a good idea to me. I will make that change. Bart.
[PATCH v3 09/15] qla2xxx: Cleanup debug message IDs
From: Quinn TranAssign unique id to all traces and logs for debug purpose. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_attr.c | 2 +- drivers/scsi/qla2xxx/qla_bsg.c| 2 +- drivers/scsi/qla2xxx/qla_dbg.c| 2 +- drivers/scsi/qla2xxx/qla_dfs.c| 8 +- drivers/scsi/qla2xxx/qla_gs.c | 100 +++-- drivers/scsi/qla2xxx/qla_init.c | 290 +++--- drivers/scsi/qla2xxx/qla_isr.c| 18 +-- drivers/scsi/qla2xxx/qla_mbx.c| 52 +++ drivers/scsi/qla2xxx/qla_os.c | 30 ++-- drivers/scsi/qla2xxx/qla_target.c | 217 ++-- 10 files changed, 352 insertions(+), 369 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c index 7c8d6c54ab70..a93eb42718e5 100644 --- a/drivers/scsi/qla2xxx/qla_attr.c +++ b/drivers/scsi/qla2xxx/qla_attr.c @@ -769,7 +769,7 @@ qla2x00_issue_logo(struct file *filp, struct kobject *kobj, did.b.area = (type & 0xff00) >> 8; did.b.al_pa = (type & 0x00ff); - ql_log(ql_log_info, vha, 0x70e3, "portid=%02x%02x%02x done\n", + ql_log(ql_log_info, vha, 0xd04d, "portid=%02x%02x%02x done\n", did.b.domain, did.b.area, did.b.al_pa); ql_log(ql_log_info, vha, 0x70e4, "%s: %d\n", __func__, type); diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c index ca3420de5a01..e093795a0371 100644 --- a/drivers/scsi/qla2xxx/qla_bsg.c +++ b/drivers/scsi/qla2xxx/qla_bsg.c @@ -2135,7 +2135,7 @@ qla8044_serdes_op(struct bsg_job *bsg_job) bsg_reply->reply_payload_rcv_len = sizeof(sr); break; default: - ql_dbg(ql_dbg_user, vha, 0x70cf, + ql_dbg(ql_dbg_user, vha, 0x7020, "Unknown serdes cmd %x.\n", sr.cmd); rval = -EINVAL; break; diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c index 88748a6ab73f..11e097e123bd 100644 --- a/drivers/scsi/qla2xxx/qla_dbg.c +++ b/drivers/scsi/qla2xxx/qla_dbg.c @@ -62,7 +62,7 @@ * | Misc | 0xd301 | 0xd031-0xd0ff | * | || 0xd101-0xd1fe | * | || 0xd214-0xd2fe | - * | Target Mode | 0xe080 || + * | Target Mode | 0xe081 || * | Target Mode Management | 0xf09b | 0xf002 | * | || 0xf046-0xf049 | * | Target Mode Task Management | 0x1000d || diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c index 989e17b0758c..391c50be2297 100644 --- a/drivers/scsi/qla2xxx/qla_dfs.c +++ b/drivers/scsi/qla2xxx/qla_dfs.c @@ -70,7 +70,7 @@ qla2x00_dfs_tgt_port_database_show(struct seq_file *s, void *unused) qla2x00_gid_list_size(ha), _list_dma, GFP_KERNEL); if (!gid_list) { - ql_dbg(ql_dbg_user, vha, 0x705c, + ql_dbg(ql_dbg_user, vha, 0x7018, "DMA allocation failed for %u\n", qla2x00_gid_list_size(ha)); return 0; @@ -370,7 +370,7 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha) ha->tgt.dfs_tgt_port_database = debugfs_create_file("tgt_port_database", S_IRUSR, ha->dfs_dir, vha, _tgt_port_database_ops); if (!ha->tgt.dfs_tgt_port_database) { - ql_log(ql_log_warn, vha, 0x, + ql_log(ql_log_warn, vha, 0xd03f, "Unable to create debugFS tgt_port_database node.\n"); goto out; } @@ -386,8 +386,8 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha) ha->tgt.dfs_tgt_sess = debugfs_create_file("tgt_sess", S_IRUSR, ha->dfs_dir, vha, _tgt_sess_ops); if (!ha->tgt.dfs_tgt_sess) { - ql_log(ql_log_warn, vha, 0x, - "Unable to create debugFS tgt_sess node.\n"); + ql_log(ql_log_warn, vha, 0xd040, + "Unable to create debugFS tgt_sess node.\n"); goto out; } diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index ef8e8891d54f..540fec524ccb 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -2024,7 +2024,7 @@ qla2x00_fdmiv2_rhba(scsi_qla_host_t *vha) eiter->len = cpu_to_be16(4 + alen); size += 4 + alen; - ql_dbg(ql_dbg_disc, vha, 0x20b1, + ql_dbg(ql_dbg_disc, vha, 0x201b, "Vendor Identifier = %s.\n", eiter->a.vendor_identifier); /* Update MS request size. */ @@ -2236,7 +2236,7 @@ qla2x00_fdmiv2_rpa(scsi_qla_host_t *vha)
[PATCH v3 14/15] qla2xxx: Remove unused irq_cmd_count field.
From: Quinn TranWhen driver is unloaded, all sessions are torn down, all commmands are flushed, chip is reset to ensure there is no knowledge of target mode in ISP. The irq_cmd_count field was used to make sure all commands are processed on top of that. The irq_cmd_count is now redundant and not needed. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_target.c | 9 + drivers/scsi/qla2xxx/qla_target.h | 1 - 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index f9ccf845d084..a8e57072019b 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -5355,8 +5355,6 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt) * Otherwise, some commands can stuck. */ - tgt->irq_cmd_count++; - switch (pkt->entry_type) { case CTIO_CRC2: case CTIO_TYPE7: @@ -5382,10 +5380,8 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt) } rc = qlt_chk_qfull_thresh_hold(vha, atio, true); - if (rc != 0) { - tgt->irq_cmd_count--; + if (rc != 0) return; - } rc = qlt_handle_cmd_for_atio(vha, atio); if (unlikely(rc != 0)) { @@ -5517,7 +5513,6 @@ static void qlt_response_pkt(struct scsi_qla_host *vha, response_t *pkt) break; } - tgt->irq_cmd_count--; } /* @@ -5547,7 +5542,6 @@ void qlt_async_event(uint16_t code, struct scsi_qla_host *vha, * Otherwise, some commands can stuck. */ - tgt->irq_cmd_count++; switch (code) { case MBA_RESET: /* Reset */ @@ -5635,7 +5629,6 @@ void qlt_async_event(uint16_t code, struct scsi_qla_host *vha, break; } - tgt->irq_cmd_count--; } static fc_port_t *qlt_get_port_database(struct scsi_qla_host *vha, diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h index 07ff565485b7..c328a267c4c3 100644 --- a/drivers/scsi/qla2xxx/qla_target.h +++ b/drivers/scsi/qla2xxx/qla_target.h @@ -790,7 +790,6 @@ struct qla_tgt { * because req_pkt() can drop/reaquire HW lock inside. Protected by * HW lock. */ - int irq_cmd_count; int atio_irq_cmd_count; int datasegs_per_cmd, datasegs_per_cont, sg_tablesize; -- 2.12.0
[PATCH v3 12/15] qla2xxx: Remove redundant wait when target is stopped.
From: Quinn TranCurrent code already destroy all target sessions when target Mode is stopped. Target core would waits for all commands that belong to each session to purge. The extra wait for interrupts to settle down is not relevant. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_target.c | 33 - 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 324048476d9e..f9ccf845d084 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1427,6 +1427,8 @@ int qlt_stop_phase1(struct qla_tgt *tgt) if (npiv_vports) { mutex_unlock(_tgt_mutex); + ql_dbg(ql_dbg_tgt_mgt, vha, 0xf021, + "NPIV is in use. Can not stop target\n"); return -EPERM; } } @@ -1437,7 +1439,7 @@ int qlt_stop_phase1(struct qla_tgt *tgt) return -EPERM; } - ql_dbg(ql_dbg_tgt, vha, 0xe003, "Stopping target for host %ld(%p)\n", + ql_dbg(ql_dbg_tgt_mgt, vha, 0xe003, "Stopping target for host %ld(%p)\n", vha->host_no, vha); /* * Mutex needed to sync with qla_tgt_fc_port_[added,deleted]. @@ -1480,9 +1482,7 @@ EXPORT_SYMBOL(qlt_stop_phase1); /* Called by tcm_qla2xxx configfs code */ void qlt_stop_phase2(struct qla_tgt *tgt) { - struct qla_hw_data *ha = tgt->ha; - scsi_qla_host_t *vha = pci_get_drvdata(ha->pdev); - unsigned long flags; + scsi_qla_host_t *vha = tgt->vha; if (tgt->tgt_stopped) { ql_dbg(ql_dbg_tgt_mgt, vha, 0xf04f, @@ -1490,24 +1490,19 @@ void qlt_stop_phase2(struct qla_tgt *tgt) dump_stack(); return; } - - ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00b, - "Waiting for %d IRQ commands to complete (tgt %p)", - tgt->irq_cmd_count, tgt); + if (!tgt->tgt_stop) { + ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00b, + "%s: phase1 stop is not completed\n", __func__); + dump_stack(); + return; + } mutex_lock(>vha_tgt.tgt_mutex); - spin_lock_irqsave(>hardware_lock, flags); - while ((tgt->irq_cmd_count != 0) || (tgt->atio_irq_cmd_count != 0)) { - spin_unlock_irqrestore(>hardware_lock, flags); - udelay(2); - spin_lock_irqsave(>hardware_lock, flags); - } tgt->tgt_stop = 0; tgt->tgt_stopped = 1; - spin_unlock_irqrestore(>hardware_lock, flags); mutex_unlock(>vha_tgt.tgt_mutex); - ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00c, "Stop of tgt %p finished", + ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00c, "Stop of tgt %p finished\n", tgt); } EXPORT_SYMBOL(qlt_stop_phase2); @@ -1517,6 +1512,10 @@ static void qlt_release(struct qla_tgt *tgt) { scsi_qla_host_t *vha = tgt->vha; + if ((vha->vha_tgt.qla_tgt != NULL) && !tgt->tgt_stop && + !tgt->tgt_stopped) + qlt_stop_phase1(tgt); + if ((vha->vha_tgt.qla_tgt != NULL) && !tgt->tgt_stopped) qlt_stop_phase2(tgt); @@ -5531,7 +5530,7 @@ void qlt_async_event(uint16_t code, struct scsi_qla_host *vha, struct qla_tgt *tgt = vha->vha_tgt.qla_tgt; int login_code; - if (!ha->tgt.tgt_ops) + if (!tgt || tgt->tgt_stop || tgt->tgt_stopped) return; if (unlikely(tgt == NULL)) { -- 2.12.0
[PATCH v3 15/15] qla2xxx: Remove extra register read
From: Quinn TranRemove extra register read in each interrupt processing to improve performance. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_iocb.c | 4 +++- drivers/scsi/qla2xxx/qla_target.c | 11 ++- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c index ea027f6a7fd4..8404f17f3c6c 100644 --- a/drivers/scsi/qla2xxx/qla_iocb.c +++ b/drivers/scsi/qla2xxx/qla_iocb.c @@ -464,7 +464,9 @@ qla2x00_start_iocbs(struct scsi_qla_host *vha, struct req_que *req) req->ring_ptr++; /* Set chip new ring index. */ - if (ha->mqenable || IS_QLA83XX(ha) || IS_QLA27XX(ha)) { + if (ha->mqenable || IS_QLA27XX(ha)) { + WRT_REG_DWORD(req->req_q_in, req->ring_index); + } else if (IS_QLA83XX(ha)) { WRT_REG_DWORD(req->req_q_in, req->ring_index); RD_REG_DWORD_RELAXED(>iobase->isp24.hccr); } else if (IS_QLAFX00(ha)) { diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index a8e57072019b..88eea4d34487 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -2252,11 +2252,10 @@ static void qlt_unmap_sg(struct scsi_qla_host *vha, struct qla_tgt_cmd *cmd) static int qlt_check_reserve_free_req(struct scsi_qla_host *vha, uint32_t req_cnt) { - uint32_t cnt, cnt_in; + uint32_t cnt; if (vha->req->cnt < (req_cnt + 2)) { cnt = (uint16_t)RD_REG_DWORD(vha->req->req_q_out); - cnt_in = (uint16_t)RD_REG_DWORD(vha->req->req_q_in); if (vha->req->ring_index < cnt) vha->req->cnt = cnt - vha->req->ring_index; @@ -2264,14 +2263,8 @@ static int qlt_check_reserve_free_req(struct scsi_qla_host *vha, vha->req->cnt = vha->req->length - (vha->req->ring_index - cnt); - if (unlikely(vha->req->cnt < (req_cnt + 2))) { - ql_dbg(ql_dbg_io, vha, 0x305a, - "qla_target(%d): There is no room in the request ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d Req-Length=%d\n", - vha->vp_idx, vha->req->ring_index, - vha->req->cnt, req_cnt, cnt, cnt_in, - vha->req->length); + if (unlikely(vha->req->cnt < (req_cnt + 2))) return -EAGAIN; - } } vha->req->cnt -= req_cnt; -- 2.12.0
[PATCH v3 05/15] tcm_qla2xxx: Do not allow aborted cmd to advance.
From: Quinn TranIn case of hardware queue full, commands can loop between TCM stack and tcm_qla2xx shim layers for retry. While command is waiting for retry, task mgmt can get ahead and abort the cmmand that encountered queue full condition. Fix this by dropping the command, if task mgmt has already started the command free process. Acked-by: Nicholas Bellinger Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 7443e4efa3ae..1131fe8e2dd2 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -686,6 +686,19 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd) struct qla_tgt_cmd, se_cmd); int xmit_type = QLA_TGT_XMIT_STATUS; + if (cmd->aborted) { + /* +* Cmd can loop during Q-full. tcm_qla2xxx_aborted_task +* can get ahead of this cmd. tcm_qla2xxx_aborted_task +* already kick start the free. +*/ + pr_debug( + "queue_data_in aborted cmd[%p] refcount %d transport_state %x, t_state %x, se_cmd_flags %x\n", + cmd, kref_read(>se_cmd.cmd_kref), + cmd->se_cmd.transport_state, cmd->se_cmd.t_state, + cmd->se_cmd.se_cmd_flags); + return 0; + } cmd->bufflen = se_cmd->data_length; cmd->sg = NULL; cmd->sg_cnt = 0; -- 2.12.0
[PATCH v3 13/15] qla2xxx: Accelerate SCSI BUSY status generation in target mode
From: Quinn TranAccelerate generation of SCSI busy to let initiators slow down when target is running low in resources. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 13 +++-- drivers/scsi/qla2xxx/qla_mbx.c | 2 ++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 08000aebe8d4..436968ad4484 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -7374,10 +7374,19 @@ qla81xx_update_fw_options(scsi_qla_host_t *vha) } if (qla_tgt_mode_enabled(vha) || - qla_dual_mode_enabled(vha)) + qla_dual_mode_enabled(vha)) { + /* FW auto send SCSI status during */ + ha->fw_options[1] |= BIT_8; + ha->fw_options[10] |= (u16)SAM_STAT_BUSY << 8; + + /* FW perform Exchange validation */ ha->fw_options[2] |= BIT_4; - else + } else { + ha->fw_options[1] &= ~BIT_8; + ha->fw_options[10] &= 0x00ff; + ha->fw_options[2] &= ~BIT_4; + } if (ql2xetsenable) { /* Enable ETS Burst. */ diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index bebac42d9e9e..f02a2baffb5b 100644 --- a/drivers/scsi/qla2xxx/qla_mbx.c +++ b/drivers/scsi/qla2xxx/qla_mbx.c @@ -1048,6 +1048,8 @@ qla2x00_set_fw_options(scsi_qla_host_t *vha, uint16_t *fwopts) mcp->in_mb = MBX_0; if (IS_FWI2_CAPABLE(vha->hw)) { mcp->in_mb |= MBX_1; + mcp->mb[10] = fwopts[10]; + mcp->out_mb |= MBX_10; } else { mcp->mb[10] = fwopts[10]; mcp->mb[11] = fwopts[11]; -- 2.12.0
[PATCH v3 11/15] qla2xxx: Add ql2xiniexchg parameter
From: Quinn TranPreviously, the ql2xexchoffld module parameter was used to control the max number of exchanges to be offload onto host memory. Module parameter ql_dm_tgt_ex_pct was used to control the percentage of exchanges allocated to the Target side. With this patch, module parameter ql_dm_tgt_ex_pct is no longer used to control exchanges for the driver. New module parameter ql2xiniexchg is added to control exchanges between target mode and initiator mode. With the updated module parameters, users can control the exact number of exchanges for either Initiator or Target. The exchange offload feature will be automatically enabled when the total number of exchanges exceeds 2048 limit. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h| 6 +- drivers/scsi/qla2xxx/qla_gbl.h| 3 +- drivers/scsi/qla2xxx/qla_init.c | 2 +- drivers/scsi/qla2xxx/qla_inline.h | 16 + drivers/scsi/qla2xxx/qla_mbx.c| 14 ++-- drivers/scsi/qla2xxx/qla_os.c | 136 +++--- drivers/scsi/qla2xxx/qla_target.c | 48 +++--- 7 files changed, 140 insertions(+), 85 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 51b262b236b4..ddf93efe3986 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -286,7 +286,7 @@ struct name_list_extended { #define RESPONSE_ENTRY_CNT_MQ 128 /* Number of response entries.*/ #define ATIO_ENTRY_CNT_24XX4096/* Number of ATIO entries. */ #define RESPONSE_ENTRY_CNT_FX00256 /* Number of response entries.*/ -#define EXTENDED_EXCH_ENTRY_CNT32768 /* Entries for offload case */ +#define FW_DEF_EXCHANGES_CNT 2048 struct req_que; struct qla_tgt_sess; @@ -3593,6 +3593,10 @@ struct qla_hw_data { #define IS_SHADOW_REG_CAPABLE(ha) (IS_QLA27XX(ha)) #define IS_DPORT_CAPABLE(ha) (IS_QLA83XX(ha) || IS_QLA27XX(ha)) #define IS_FAWWN_CAPABLE(ha) (IS_QLA83XX(ha) || IS_QLA27XX(ha)) +#define IS_EXCHG_OFFLD_CAPABLE(ha) \ + (IS_QLA81XX(ha) || IS_QLA83XX(ha) || IS_QLA27XX(ha)) +#define IS_EXLOGIN_OFFLD_CAPABLE(ha) \ + (IS_QLA25XX(ha) || IS_QLA81XX(ha) || IS_QLA83XX(ha) || IS_QLA27XX(ha)) /* HBA serial number */ uint8_t serial0; diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h index 5b2451745e9f..f8540f5c9e5d 100644 --- a/drivers/scsi/qla2xxx/qla_gbl.h +++ b/drivers/scsi/qla2xxx/qla_gbl.h @@ -137,6 +137,7 @@ extern int ql2xmdcapmask; extern int ql2xmdenable; extern int ql2xexlogins; extern int ql2xexchoffld; +extern int ql2xiniexchg; extern int ql2xfwholdabts; extern int ql2xmvasynctoatio; @@ -839,7 +840,7 @@ extern int qla_get_exlogin_status(scsi_qla_host_t *, uint16_t *, uint16_t *); extern int qla_set_exlogin_mem_cfg(scsi_qla_host_t *vha, dma_addr_t phys_addr); extern int qla_get_exchoffld_status(scsi_qla_host_t *, uint16_t *, uint16_t *); -extern int qla_set_exchoffld_mem_cfg(scsi_qla_host_t *, dma_addr_t); +extern int qla_set_exchoffld_mem_cfg(scsi_qla_host_t *); extern void qlt_handle_abts_recv(struct scsi_qla_host *, response_t *); int qla24xx_async_notify_ack(scsi_qla_host_t *, fc_port_t *, diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index f92e74639bb1..08000aebe8d4 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -2723,7 +2723,7 @@ qla2x00_setup_chip(scsi_qla_host_t *vha) if (ql2xexlogins) ha->flags.exlogins_enabled = 1; - if (ql2xexchoffld) + if (qla_is_exch_offld_enabled(vha)) ha->flags.exchoffld_enabled = 1; rval = qla2x00_execute_fw(vha, srisc_address); diff --git a/drivers/scsi/qla2xxx/qla_inline.h b/drivers/scsi/qla2xxx/qla_inline.h index c61a6a871c8e..9996ec0daab1 100644 --- a/drivers/scsi/qla2xxx/qla_inline.h +++ b/drivers/scsi/qla2xxx/qla_inline.h @@ -307,3 +307,19 @@ qla2x00_set_retry_delay_timestamp(fc_port_t *fcport, uint16_t retry_delay) fcport->retry_delay_timestamp = jiffies + (retry_delay * HZ / 10); } + +static inline bool +qla_is_exch_offld_enabled(struct scsi_qla_host *vha) +{ + if (qla_ini_mode_enabled(vha) && + (ql2xiniexchg > FW_DEF_EXCHANGES_CNT)) + return true; + else if (qla_tgt_mode_enabled(vha) && + (ql2xexchoffld > FW_DEF_EXCHANGES_CNT)) + return true; + else if (qla_dual_mode_enabled(vha) && + ((ql2xiniexchg + ql2xexchoffld) > FW_DEF_EXCHANGES_CNT)) + return true; + else + return false; +} diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index
[PATCH v3 10/15] qla2xxx: Turn on FW option for exchange check
From: Quinn TranTell FW to track exchange/cmd state to prevent driver from using stale exchange or exchange that is not meant for this command. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_init.c | 18 ++ drivers/scsi/qla2xxx/qla_target.c | 26 +++--- drivers/scsi/qla2xxx/qla_target.h | 2 +- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index c425d061cd80..f92e74639bb1 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -2969,6 +2969,18 @@ qla24xx_update_fw_options(scsi_qla_host_t *vha) ha->fw_options[2] &= ~BIT_11; } + if (IS_QLA25XX(ha) || IS_QLA83XX(ha) || IS_QLA27XX(ha)) { + /* +* Tell FW to track each exchange to prevent +* driver from using stale exchange. +*/ + if (qla_tgt_mode_enabled(vha) || + qla_dual_mode_enabled(vha)) + ha->fw_options[2] |= BIT_4; + else + ha->fw_options[2] &= ~BIT_4; + } + ql_dbg(ql_dbg_init, vha, 0x00e8, "%s, add FW options 1-3 = 0x%04x 0x%04x 0x%04x mode %x\n", __func__, ha->fw_options[1], ha->fw_options[2], @@ -7361,6 +7373,12 @@ qla81xx_update_fw_options(scsi_qla_host_t *vha) ha->fw_options[2] &= ~BIT_11; } + if (qla_tgt_mode_enabled(vha) || + qla_dual_mode_enabled(vha)) + ha->fw_options[2] |= BIT_4; + else + ha->fw_options[2] &= ~BIT_4; + if (ql2xetsenable) { /* Enable ETS Burst. */ memset(ha->fw_options, 0, sizeof(ha->fw_options)); diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 4ad09584d4a8..3cdffdd90279 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -2016,8 +2016,9 @@ static void qlt_24xx_send_task_mgmt_ctio(struct scsi_qla_host *ha, ctio->initiator_id[1] = atio->u.isp24.fcp_hdr.s_id[1]; ctio->initiator_id[2] = atio->u.isp24.fcp_hdr.s_id[0]; ctio->exchange_addr = atio->u.isp24.exchange_addr; - ctio->u.status1.flags = (atio->u.isp24.attr << 9) | - cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_SEND_STATUS); + temp = (atio->u.isp24.attr << 9)| + CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_SEND_STATUS; + ctio->u.status1.flags = cpu_to_le16(temp); temp = be16_to_cpu(atio->u.isp24.fcp_hdr.ox_id); ctio->u.status1.ox_id = cpu_to_le16(temp); ctio->u.status1.scsi_status = @@ -2070,8 +2071,9 @@ void qlt_send_resp_ctio(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd, ctio->initiator_id[1] = atio->u.isp24.fcp_hdr.s_id[1]; ctio->initiator_id[2] = atio->u.isp24.fcp_hdr.s_id[0]; ctio->exchange_addr = atio->u.isp24.exchange_addr; - ctio->u.status1.flags = (atio->u.isp24.attr << 9) | - cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_SEND_STATUS); + temp = (atio->u.isp24.attr << 9) | + CTIO7_FLAGS_STATUS_MODE_1 | CTIO7_FLAGS_SEND_STATUS; + ctio->u.status1.flags = cpu_to_le16(temp); temp = be16_to_cpu(atio->u.isp24.fcp_hdr.ox_id); ctio->u.status1.ox_id = cpu_to_le16(temp); ctio->u.status1.scsi_status = @@ -2359,7 +2361,8 @@ static int qlt_24xx_build_ctio_pkt(struct qla_tgt_prm *prm, pkt->initiator_id[1] = atio->u.isp24.fcp_hdr.s_id[1]; pkt->initiator_id[2] = atio->u.isp24.fcp_hdr.s_id[0]; pkt->exchange_addr = atio->u.isp24.exchange_addr; - pkt->u.status0.flags |= (atio->u.isp24.attr << 9); + temp = atio->u.isp24.attr << 9; + pkt->u.status0.flags |= cpu_to_le16(temp); temp = be16_to_cpu(atio->u.isp24.fcp_hdr.ox_id); pkt->u.status0.ox_id = cpu_to_le16(temp); pkt->u.status0.relative_offset = cpu_to_le32(prm->cmd->offset); @@ -3484,9 +3487,9 @@ static int __qlt_send_term_exchange(struct scsi_qla_host *vha, ctio24->initiator_id[1] = atio->u.isp24.fcp_hdr.s_id[1]; ctio24->initiator_id[2] = atio->u.isp24.fcp_hdr.s_id[0]; ctio24->exchange_addr = atio->u.isp24.exchange_addr; - ctio24->u.status1.flags = (atio->u.isp24.attr << 9) | - cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 | - CTIO7_FLAGS_TERMINATE); + temp = (atio->u.isp24.attr << 9) | CTIO7_FLAGS_STATUS_MODE_1 | + CTIO7_FLAGS_TERMINATE; + ctio24->u.status1.flags = cpu_to_le16(temp); temp = be16_to_cpu(atio->u.isp24.fcp_hdr.ox_id); ctio24->u.status1.ox_id = cpu_to_le16(temp); @@ -4978,6 +4981,7 @@ static int __qlt_send_busy(struct scsi_qla_host *vha, request_t *pkt; struct fc_port
[PATCH v3 03/15] qla2xxx: Retain loop test for fwdump length exceeding buffer length
From: Joe CarnuccioSigned-off-by: Joe Carnuccio Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/qla_init.c | 8 drivers/scsi/qla2xxx/qla_tmpl.c | 16 +--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index f6130e8b1ca1..e4876f4220e4 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -6356,8 +6356,8 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t *srisc_addr, "-> template size %x bytes\n", dlen); if (dlen > risc_size * sizeof(*dcode)) { ql_log(ql_log_warn, vha, 0x0167, - "Failed fwdump template exceeds array by %x bytes\n", - (uint32_t)(dlen - risc_size * sizeof(*dcode))); + "Failed fwdump template exceeds array by %lx bytes\n", + (size_t)(dlen - risc_size * sizeof(*dcode))); goto default_template; } ha->fw_dump_template_len = dlen; @@ -6658,8 +6658,8 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t *srisc_addr) "-> template size %x bytes\n", dlen); if (dlen > risc_size * sizeof(*fwcode)) { ql_log(ql_log_warn, vha, 0x0177, - "Failed fwdump template exceeds array by %x bytes\n", - (uint32_t)(dlen - risc_size * sizeof(*fwcode))); + "Failed fwdump template exceeds array by %lx bytes\n", + (size_t)(dlen - risc_size * sizeof(*fwcode))); goto default_template; } ha->fw_dump_template_len = dlen; diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c index c197972a3e2d..33142610882f 100644 --- a/drivers/scsi/qla2xxx/qla_tmpl.c +++ b/drivers/scsi/qla2xxx/qla_tmpl.c @@ -219,8 +219,6 @@ qla27xx_skip_entry(struct qla27xx_fwdt_entry *ent, void *buf) { if (buf) ent->hdr.driver_flags |= DRIVER_FLAG_SKIP_ENTRY; - ql_dbg(ql_dbg_misc + ql_dbg_verbose, NULL, 0xd011, - "Skipping entry %d\n", ent->hdr.entry_type); } static int @@ -818,6 +816,8 @@ qla27xx_walk_template(struct scsi_qla_host *vha, ql_dbg(ql_dbg_misc, vha, 0xd01a, "%s: entry count %lx\n", __func__, count); while (count--) { + if (buf && *len >= vha->hw->fw_dump_len) + break; if (qla27xx_find_entry(ent->hdr.entry_type)(vha, ent, buf, len)) break; ent = qla27xx_next_entry(ent); @@ -825,18 +825,20 @@ qla27xx_walk_template(struct scsi_qla_host *vha, if (count) ql_dbg(ql_dbg_misc, vha, 0xd018, - "%s: residual count (%lx)\n", __func__, count); + "%s: entry residual count (%lx)\n", __func__, count); if (ent->hdr.entry_type != ENTRY_TYPE_TMP_END) ql_dbg(ql_dbg_misc, vha, 0xd019, - "%s: missing end (%lx)\n", __func__, count); + "%s: missing end entry (%lx)\n", __func__, count); - ql_dbg(ql_dbg_misc, vha, 0xd01b, - "%s: len=%lx\n", __func__, *len); + if (buf && *len != vha->hw->fw_dump_len) + ql_dbg(ql_dbg_misc, vha, 0xd01b, + "%s: length=%#lx residual=%+ld\n", + __func__, *len, vha->hw->fw_dump_len - *len); if (buf) { ql_log(ql_log_warn, vha, 0xd015, - "Firmware dump saved to temp buffer (%ld/%p)\n", + "Firmware dump saved to temp buffer (%lu/%p)\n", vha->host_no, vha->hw->fw_dump); qla2x00_post_uevent_work(vha, QLA_UEVENT_CODE_FW_DUMP); } -- 2.12.0
[PATCH v3 07/15] qla2xxx: Convert 32-bit LUN usage to 64-bit
From: Quinn TranAcked-by: Nicholas Bellinger Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/qla_target.c | 32 +++- drivers/scsi/qla2xxx/qla_target.h | 4 ++-- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index a2e17a5794ab..a1f33b06019d 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1799,7 +1799,7 @@ static int abort_cmd_for_tag(struct scsi_qla_host *vha, uint32_t tag) * for the same lun) */ static void abort_cmds_for_lun(struct scsi_qla_host *vha, - uint32_t lun, uint8_t *s_id) + u64 lun, uint8_t *s_id) { struct qla_tgt_sess_op *op; struct qla_tgt_cmd *cmd; @@ -1810,7 +1810,7 @@ static void abort_cmds_for_lun(struct scsi_qla_host *vha, spin_lock_irqsave(>cmd_list_lock, flags); list_for_each_entry(op, >qla_sess_op_cmd_list, cmd_list) { uint32_t op_key; - uint32_t op_lun; + u64 op_lun; op_key = sid_to_key(op->atio.u.isp24.fcp_hdr.s_id); op_lun = scsilun_to_int( @@ -1832,7 +1832,7 @@ static void abort_cmds_for_lun(struct scsi_qla_host *vha, list_for_each_entry(cmd, >qla_cmd_list, cmd_list) { uint32_t cmd_key; - uint32_t cmd_lun; + u64 cmd_lun; cmd_key = sid_to_key(cmd->atio.u.isp24.fcp_hdr.s_id); cmd_lun = scsilun_to_int( @@ -1850,18 +1850,15 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, struct qla_hw_data *ha = vha->hw; struct se_session *se_sess = sess->se_sess; struct qla_tgt_mgmt_cmd *mcmd; + struct qla_tgt_cmd *cmd; struct se_cmd *se_cmd; - u32 lun = 0; int rc; bool found_lun = false; unsigned long flags; spin_lock_irqsave(_sess->sess_cmd_lock, flags); list_for_each_entry(se_cmd, _sess->sess_cmd_list, se_cmd_list) { - struct qla_tgt_cmd *cmd = - container_of(se_cmd, struct qla_tgt_cmd, se_cmd); if (se_cmd->tag == abts->exchange_addr_to_abort) { - lun = cmd->unpacked_lun; found_lun = true; break; } @@ -1895,12 +1892,13 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, } memset(mcmd, 0, sizeof(*mcmd)); + cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd); mcmd->sess = sess; memcpy(>orig_iocb.abts, abts, sizeof(mcmd->orig_iocb.abts)); mcmd->reset_count = vha->hw->chip_reset; mcmd->tmr_func = QLA_TGT_ABTS; - rc = ha->tgt.tgt_ops->handle_tmr(mcmd, lun, mcmd->tmr_func, + rc = ha->tgt.tgt_ops->handle_tmr(mcmd, cmd->unpacked_lun, mcmd->tmr_func, abts->exchange_addr_to_abort); if (rc != 0) { ql_dbg(ql_dbg_tgt_mgt, vha, 0xf052, @@ -4334,13 +4332,12 @@ static int qlt_handle_task_mgmt(struct scsi_qla_host *vha, void *iocb) struct qla_hw_data *ha = vha->hw; struct qla_tgt *tgt; struct fc_port *sess; - uint32_t lun, unpacked_lun; + u64 unpacked_lun; int fn; unsigned long flags; tgt = vha->vha_tgt.qla_tgt; - lun = a->u.isp24.fcp_cmnd.lun; fn = a->u.isp24.fcp_cmnd.task_mgmt_flags; spin_lock_irqsave(>tgt.sess_lock, flags); @@ -4348,7 +4345,8 @@ static int qlt_handle_task_mgmt(struct scsi_qla_host *vha, void *iocb) a->u.isp24.fcp_hdr.s_id); spin_unlock_irqrestore(>tgt.sess_lock, flags); - unpacked_lun = scsilun_to_int((struct scsi_lun *)); + unpacked_lun = + scsilun_to_int((struct scsi_lun *)>u.isp24.fcp_cmnd.lun); if (!sess) { ql_dbg(ql_dbg_tgt_mgt, vha, 0xf024, @@ -4371,7 +4369,7 @@ static int __qlt_abort_task(struct scsi_qla_host *vha, struct atio_from_isp *a = (struct atio_from_isp *)iocb; struct qla_hw_data *ha = vha->hw; struct qla_tgt_mgmt_cmd *mcmd; - uint32_t lun, unpacked_lun; + u64 unpacked_lun; int rc; mcmd = mempool_alloc(qla_tgt_mgmt_cmd_mempool, GFP_ATOMIC); @@ -4387,8 +4385,8 @@ static int __qlt_abort_task(struct scsi_qla_host *vha, memcpy(>orig_iocb.imm_ntfy, iocb, sizeof(mcmd->orig_iocb.imm_ntfy)); - lun = a->u.isp24.fcp_cmnd.lun; - unpacked_lun = scsilun_to_int((struct scsi_lun *)); + unpacked_lun = + scsilun_to_int((struct scsi_lun *)>u.isp24.fcp_cmnd.lun); mcmd->reset_count = vha->hw->chip_reset;
[PATCH v3 08/15] qla2xxx: Fix name server relogin
From: Quinn TranName server login is normally handle by FW. In some rare case where one of the switches is being updated, name server login could get affected. Trigger relogin to name server when driver detects this condition. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 2 ++ drivers/scsi/qla2xxx/qla_gs.c | 20 drivers/scsi/qla2xxx/qla_init.c | 38 +- drivers/scsi/qla2xxx/qla_isr.c | 17 + 4 files changed, 76 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 4127f35b669c..51b262b236b4 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -252,6 +252,8 @@ #define NPH_F_PORT 0x7fe /* FE */ #define NPH_IP_BROADCAST 0x7ff /* FF */ +#define NPH_SNS_LID(ha)(IS_FWI2_CAPABLE(ha) ? NPH_SNS : SIMPLE_NAME_SERVER) + #define MAX_CMDSZ 16 /* SCSI maximum CDB size. */ #include "qla_fw.h" diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 5acebaf57796..ef8e8891d54f 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -124,6 +124,7 @@ qla2x00_chk_ms_status(scsi_qla_host_t *vha, ms_iocb_entry_t *ms_pkt, int rval; uint16_t comp_status; struct qla_hw_data *ha = vha->hw; + bool lid_is_sns = false; rval = QLA_FUNCTION_FAILED; if (ms_pkt->entry_status != 0) { @@ -155,6 +156,25 @@ qla2x00_chk_ms_status(scsi_qla_host_t *vha, ms_iocb_entry_t *ms_pkt, } else rval = QLA_SUCCESS; break; + case CS_PORT_LOGGED_OUT: + if (IS_FWI2_CAPABLE(ha)) { + if (le16_to_cpu(ms_pkt->loop_id.extended) == + NPH_SNS) + lid_is_sns = true; + } else { + if (le16_to_cpu(ms_pkt->loop_id.extended) == + SIMPLE_NAME_SERVER) + lid_is_sns = true; + } + if (lid_is_sns) { + ql_dbg(ql_dbg_async, vha, 0x502b, + "%s failed, Name server has logged out", + routine); + rval = QLA_NOT_LOGGED_IN; + set_bit(LOOP_RESYNC_NEEDED, >dpc_flags); + set_bit(LOCAL_LOOP_UPDATE, >dpc_flags); + } + break; default: ql_dbg(ql_dbg_disc, vha, 0x2033, "%s failed, completion status (%x) on port_id: " diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index e4876f4220e4..4ea4aa5bddaa 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -1041,6 +1041,20 @@ void qla2x00_fcport_event_handler(scsi_qla_host_t *vha, struct event_arg *ea) switch (ea->event) { case FCME_RELOGIN: + case FCME_RSCN: + case FCME_GIDPN_DONE: + case FCME_GPSC_DONE: + case FCME_GPNID_DONE: + if (test_bit(LOOP_RESYNC_NEEDED, >dpc_flags) || + test_bit(LOOP_RESYNC_ACTIVE, >dpc_flags)) + return; + break; + default: + break; + } + + switch (ea->event) { + case FCME_RELOGIN: if (test_bit(UNLOADING, >dpc_flags)) return; @@ -4451,20 +4465,31 @@ qla2x00_configure_fabric(scsi_qla_host_t *vha) /* EMPTY */ ql_dbg(ql_dbg_disc, vha, 0x2045, "Register FC-4 TYPE failed.\n"); + if (test_bit(LOOP_RESYNC_NEEDED, + >dpc_flags)) + break; } if (qla2x00_rff_id(vha)) { /* EMPTY */ ql_dbg(ql_dbg_disc, vha, 0x2049, "Register FC-4 Features failed.\n"); + if (test_bit(LOOP_RESYNC_NEEDED, + >dpc_flags)) + break; } if (qla2x00_rnn_id(vha)) { /* EMPTY */ ql_dbg(ql_dbg_disc, vha, 0x204f, "Register Node Name failed.\n"); +
[PATCH v3 04/15] qla2xxx: Fix path recovery
From: Quinn TranIf the port is moved/changed, current code would trigger a deletion. If the port is already deleted, then do relogin. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/qla_gs.c | 21 - 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 9bc9aa9e164a..5acebaf57796 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -3118,16 +3118,27 @@ void qla24xx_handle_gpnid_event(scsi_qla_host_t *vha, struct event_arg *ea) if (fcport) { /* cable moved. just plugged in */ - ql_dbg(ql_dbg_disc, vha, 0x, - "%s %d %8phC post del sess\n", - __func__, __LINE__, fcport->port_name); - fcport->rscn_gen++; fcport->d_id = ea->id; fcport->scan_state = QLA_FCPORT_FOUND; fcport->flags |= FCF_FABRIC_DEVICE; - qlt_schedule_sess_for_deletion_lock(fcport); + switch (fcport->disc_state) { + case DSC_DELETED: + ql_dbg(ql_dbg_disc, vha, 0x210d, + "%s %d %8phC login\n", __func__, __LINE__, + fcport->port_name); + qla24xx_fcport_handle_login(vha, fcport); + break; + case DSC_DELETE_PEND: + break; + default: + ql_dbg(ql_dbg_disc, vha, 0x2064, + "%s %d %8phC post del sess\n", + __func__, __LINE__, fcport->port_name); + qlt_schedule_sess_for_deletion_lock(fcport); + break; + } } else { /* create new fcport */ ql_dbg(ql_dbg_disc, vha, 0x, -- 2.12.0
[PATCH v3 06/15] qla2xxx: Use flag PFLG_DISCONNECTED.
From: Sawan ChandakThere is already flag defined PFLG_DISCONNECTED, which is set for PCI or register disconnect error condition. There is no need to have flag PCI_ERR, which has same purpose. Remove use of PCI_ERR flag and use PFLG_DISCONNECTED flag during error condition. Signed-off-by: Sawan Chandak Signed-off-by: Himanshu Madhani --- drivers/scsi/qla2xxx/qla_def.h | 1 - drivers/scsi/qla2xxx/qla_mbx.c | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index eddbc1218a39..4127f35b669c 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -4017,7 +4017,6 @@ typedef struct scsi_qla_host { #define PFLG_DISCONNECTED 0 /* PCI device removed */ #define PFLG_DRIVER_REMOVING 1 /* PCI driver .remove */ #define PFLG_DRIVER_PROBING2 /* PCI driver .probe */ -#define PCI_ERR30 uint32_tdevice_flags; #define SWITCH_FOUND BIT_0 diff --git a/drivers/scsi/qla2xxx/qla_mbx.c b/drivers/scsi/qla2xxx/qla_mbx.c index cba1fc5e8be9..fffa1f7cd8d2 100644 --- a/drivers/scsi/qla2xxx/qla_mbx.c +++ b/drivers/scsi/qla2xxx/qla_mbx.c @@ -124,7 +124,8 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) } /* if PCI error, then avoid mbx processing.*/ - if (test_bit(PCI_ERR, _vha->dpc_flags)) { + if (test_bit(PFLG_DISCONNECTED, _vha->dpc_flags) && + test_bit(UNLOADING, _vha->dpc_flags)) { ql_log(ql_log_warn, vha, 0x1191, "PCI error, exiting.\n"); return QLA_FUNCTION_TIMEOUT; @@ -384,8 +385,6 @@ qla2x00_mailbox_command(scsi_qla_host_t *vha, mbx_cmd_t *mcp) * then only PCI ERR flag would be set. * we will do premature exit for above case. */ - if (test_bit(UNLOADING, _vha->dpc_flags)) - set_bit(PCI_ERR, _vha->dpc_flags); ha->flags.mbox_busy = 0; rval = QLA_FUNCTION_TIMEOUT; goto premature_exit; -- 2.12.0
[PATCH v3 02/15] qla2xxx: Replace usage of spin_lock with spin_lock_irqsave
From: Quinn TranConvert usage of spin_lock to spin_lock_irqsave because qla2xxx driver can access all the data structures in an interrupt context. Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/qla_target.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index e766d8412384..a2e17a5794ab 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1762,13 +1762,13 @@ static int abort_cmd_for_tag(struct scsi_qla_host *vha, uint32_t tag) { struct qla_tgt_sess_op *op; struct qla_tgt_cmd *cmd; + unsigned long flags; - spin_lock(>cmd_list_lock); - + spin_lock_irqsave(>cmd_list_lock, flags); list_for_each_entry(op, >qla_sess_op_cmd_list, cmd_list) { if (tag == op->atio.u.isp24.exchange_addr) { op->aborted = true; - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); return 1; } } @@ -1776,7 +1776,7 @@ static int abort_cmd_for_tag(struct scsi_qla_host *vha, uint32_t tag) list_for_each_entry(op, >unknown_atio_list, cmd_list) { if (tag == op->atio.u.isp24.exchange_addr) { op->aborted = true; - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); return 1; } } @@ -1784,12 +1784,12 @@ static int abort_cmd_for_tag(struct scsi_qla_host *vha, uint32_t tag) list_for_each_entry(cmd, >qla_cmd_list, cmd_list) { if (tag == cmd->atio.u.isp24.exchange_addr) { cmd->aborted = 1; - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); return 1; } } + spin_unlock_irqrestore(>cmd_list_lock, flags); - spin_unlock(>cmd_list_lock); return 0; } @@ -1804,9 +1804,10 @@ static void abort_cmds_for_lun(struct scsi_qla_host *vha, struct qla_tgt_sess_op *op; struct qla_tgt_cmd *cmd; uint32_t key; + unsigned long flags; key = sid_to_key(s_id); - spin_lock(>cmd_list_lock); + spin_lock_irqsave(>cmd_list_lock, flags); list_for_each_entry(op, >qla_sess_op_cmd_list, cmd_list) { uint32_t op_key; uint32_t op_lun; @@ -1839,7 +1840,7 @@ static void abort_cmds_for_lun(struct scsi_qla_host *vha, if (cmd_key == key && cmd_lun == lun) cmd->aborted = 1; } - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); } /* ha->hardware_lock supposed to be held on entry */ @@ -4216,9 +4217,9 @@ static int qlt_handle_cmd_for_atio(struct scsi_qla_host *vha, memcpy(>atio, atio, sizeof(*atio)); op->vha = vha; - spin_lock(>cmd_list_lock); + spin_lock_irqsave(>cmd_list_lock, flags); list_add_tail(>cmd_list, >qla_sess_op_cmd_list); - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); INIT_WORK(>work, qlt_create_sess_from_atio); queue_work(qla_tgt_wq, >work); @@ -4529,12 +4530,13 @@ static int abort_cmds_for_s_id(struct scsi_qla_host *vha, port_id_t *s_id) struct qla_tgt_cmd *cmd; uint32_t key; int count = 0; + unsigned long flags; key = (((u32)s_id->b.domain << 16) | ((u32)s_id->b.area << 8) | ((u32)s_id->b.al_pa)); - spin_lock(>cmd_list_lock); + spin_lock_irqsave(>cmd_list_lock, flags); list_for_each_entry(op, >qla_sess_op_cmd_list, cmd_list) { uint32_t op_key = sid_to_key(op->atio.u.isp24.fcp_hdr.s_id); @@ -4559,7 +4561,7 @@ static int abort_cmds_for_s_id(struct scsi_qla_host *vha, port_id_t *s_id) count++; } } - spin_unlock(>cmd_list_lock); + spin_unlock_irqrestore(>cmd_list_lock, flags); return count; } -- 2.12.0
[PATCH v3 01/15] qla2xxx: Allow ABTS, PURX, RIDA on ATIOQ for ISP83XX/27XX
From: Quinn TranDriver added mechanism to move ABTS/PUREX/RIDA mailbox to ATIO queue as part of commit id 41dc529a4602ac737020f423f84686a81de38e6d ("qla2xxx: Improve RSCN handling in driver"). This patch adds a check to only allow ABTS/PURX/RIDA to be moved to ATIO Queue for ISP83XX and ISP27XX. Cc: # 4.11 Signed-off-by: Quinn Tran Signed-off-by: Himanshu Madhani Reviewed-by: Bart Van Assche --- drivers/scsi/qla2xxx/qla_init.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 0391fc317003..f6130e8b1ca1 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -2946,7 +2946,8 @@ qla24xx_update_fw_options(scsi_qla_host_t *vha) } /* Move PUREX, ABTS RX & RIDA to ATIOQ */ - if (ql2xmvasynctoatio) { + if (ql2xmvasynctoatio && + (IS_QLA83XX(ha) || IS_QLA27XX(ha))) { if (qla_tgt_mode_enabled(vha) || qla_dual_mode_enabled(vha)) ha->fw_options[2] |= BIT_11; @@ -2958,7 +2959,9 @@ qla24xx_update_fw_options(scsi_qla_host_t *vha) "%s, add FW options 1-3 = 0x%04x 0x%04x 0x%04x mode %x\n", __func__, ha->fw_options[1], ha->fw_options[2], ha->fw_options[3], vha->host->active_mode); - qla2x00_set_fw_options(vha, ha->fw_options); + + if (ha->fw_options[1] || ha->fw_options[2] || ha->fw_options[3]) + qla2x00_set_fw_options(vha, ha->fw_options); /* Update Serial Link options. */ if ((le16_to_cpu(ha->fw_seriallink_options24[0]) & BIT_0) == 0) -- 2.12.0
[PATCH v3 00/15] qla2xxx: Cleanup and minor fixes
Hi Martin, This series contains patches that were dropped from 4.12.0-rc3 inclusion, since they can go to 4.13 merge window. Changes from v2 --> v3 o Added Reviewed-by tag from Bart. o Droped couple patches for rework. o Addressed minor comments from Bart where applicable. Changes from v1 --> v2 o Addressed 0-day kernel warning. o Addressed cleanups and updates as per Bart's comments. o Added Acked-by tag from Nicholas to applicable patches. Please apply this series to for-next branch to be included in 4.13 merge window. Thanks, Himanshu Joe Carnuccio (1): qla2xxx: Retain loop test for fwdump length exceeding buffer length Quinn Tran (13): qla2xxx: Allow ABTS, PURX, RIDA on ATIOQ for ISP83XX/27XX qla2xxx: Replace usage of spin_lock with spin_lock_irqsave qla2xxx: Fix path recovery tcm_qla2xxx: Do not allow aborted cmd to advance. qla2xxx: Convert 32-bit LUN usage to 64-bit qla2xxx: Fix name server relogin qla2xxx: Cleanup debug message IDs qla2xxx: Turn on FW option for exchange check qla2xxx: Add ql2xiniexchg parameter qla2xxx: Remove redundant wait when target is stopped. qla2xxx: Accelerate SCSI BUSY status generation in target mode qla2xxx: Remove unused irq_cmd_count field. qla2xxx: Remove extra register read Sawan Chandak (1): qla2xxx: Use flag PFLG_DISCONNECTED. drivers/scsi/qla2xxx/qla_attr.c| 2 +- drivers/scsi/qla2xxx/qla_bsg.c | 2 +- drivers/scsi/qla2xxx/qla_dbg.c | 2 +- drivers/scsi/qla2xxx/qla_def.h | 9 +- drivers/scsi/qla2xxx/qla_dfs.c | 8 +- drivers/scsi/qla2xxx/qla_gbl.h | 3 +- drivers/scsi/qla2xxx/qla_gs.c | 141 +++-- drivers/scsi/qla2xxx/qla_init.c| 372 -- drivers/scsi/qla2xxx/qla_inline.h | 16 ++ drivers/scsi/qla2xxx/qla_iocb.c| 4 +- drivers/scsi/qla2xxx/qla_isr.c | 35 +++- drivers/scsi/qla2xxx/qla_mbx.c | 73 +++ drivers/scsi/qla2xxx/qla_os.c | 166 ++- drivers/scsi/qla2xxx/qla_target.c | 402 - drivers/scsi/qla2xxx/qla_target.h | 7 +- drivers/scsi/qla2xxx/qla_tmpl.c| 16 +- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 15 +- 17 files changed, 716 insertions(+), 557 deletions(-) -- 2.12.0
Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
On Fri, 2 Jun 2017, walter harms wrote: > > > Am 02.06.2017 14:39, schrieb Milan P. Gandhi: > > Simplify the check for return code of fcoe_if_init routine > > in fcoe_init function such that we could eliminate need for > > extra 'out_free' label and duplicate mutex_unlock statement. > > > > Signed-off-by: Milan P. Gandhi> > --- > > drivers/scsi/fcoe/fcoe.c | 7 +++ > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > > index ea21e7b..a2cf3d0 100644 > > --- a/drivers/scsi/fcoe/fcoe.c > > +++ b/drivers/scsi/fcoe/fcoe.c > > @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void) > > fcoe_dev_setup(); > > > > rc = fcoe_if_init(); > > + mutex_unlock(_config_mutex); > > + > > if (rc) > > - goto out_free; > > + goto out_destroy; > > > > - mutex_unlock(_config_mutex); > > return 0; > > > if you do that, why not > if (!rc) return 0; I agree with Dan. If's should be for failures. julia > > re, > wh > > > > > -out_free: > > - mutex_unlock(_config_mutex); > > out_destroy: > > destroy_workqueue(fcoe_wq); > > return rc; > > -- > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" > > in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
On Fri, 2 Jun 2017, Milan P. Gandhi wrote: > Simplify the check for return code of fcoe_if_init routine > in fcoe_init function such that we could eliminate need for > extra 'out_free' label and duplicate mutex_unlock statement. > > Signed-off-by: Milan P. Gandhi> --- > drivers/scsi/fcoe/fcoe.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > index ea21e7b..a2cf3d0 100644 > --- a/drivers/scsi/fcoe/fcoe.c > +++ b/drivers/scsi/fcoe/fcoe.c > @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void) > fcoe_dev_setup(); > > rc = fcoe_if_init(); > + mutex_unlock(_config_mutex); > + > if (rc) > - goto out_free; > + goto out_destroy; > > - mutex_unlock(_config_mutex); That's what I was thinking of, but it's not a RESEND, but rather a v2. You need to explain under the --- what is the change since the original submission. julia > return 0; > > -out_free: > - mutex_unlock(_config_mutex); > out_destroy: > destroy_workqueue(fcoe_wq); > return rc; >
Re: [PATCH net-next 0/4] qed: Enhance storage APIs
From: Yuval MintzDate: Fri, 2 Jun 2017 08:58:29 +0300 > This series is intended to add additional information and features > to the API between qed and its storage protocol drivers [qedi, qedf]. > > Patch #2 adds some information stored on device such as wwpn & wwnn > to allow qedf utilize it; #1 fixes an issue with the reading of those > values [which were unused until now]. > > Patch #3 would allow the protocol drivers access to images on persistent > storage which is a prerequirement for adding boot from SAN support. > > Patch #4 adds infrastrucutre to a future feature for qedi. Series applied.
Re: [PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
Am 02.06.2017 14:39, schrieb Milan P. Gandhi: > Simplify the check for return code of fcoe_if_init routine > in fcoe_init function such that we could eliminate need for > extra 'out_free' label and duplicate mutex_unlock statement. > > Signed-off-by: Milan P. Gandhi> --- > drivers/scsi/fcoe/fcoe.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > index ea21e7b..a2cf3d0 100644 > --- a/drivers/scsi/fcoe/fcoe.c > +++ b/drivers/scsi/fcoe/fcoe.c > @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void) > fcoe_dev_setup(); > > rc = fcoe_if_init(); > + mutex_unlock(_config_mutex); > + > if (rc) > - goto out_free; > + goto out_destroy; > > - mutex_unlock(_config_mutex); > return 0; > if you do that, why not if (!rc) return 0; re, wh > -out_free: > - mutex_unlock(_config_mutex); > out_destroy: > destroy_workqueue(fcoe_wq); > return rc; > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
[PATCH RESEND] Eliminate extra 'out_free' label from fcoe_init function
Simplify the check for return code of fcoe_if_init routine in fcoe_init function such that we could eliminate need for extra 'out_free' label and duplicate mutex_unlock statement. Signed-off-by: Milan P. Gandhi--- drivers/scsi/fcoe/fcoe.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index ea21e7b..a2cf3d0 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c @@ -2523,14 +2523,13 @@ static int __init fcoe_init(void) fcoe_dev_setup(); rc = fcoe_if_init(); + mutex_unlock(_config_mutex); + if (rc) - goto out_free; + goto out_destroy; - mutex_unlock(_config_mutex); return 0; -out_free: - mutex_unlock(_config_mutex); out_destroy: destroy_workqueue(fcoe_wq); return rc;
Re: [PATCH] Eliminate extra 'out_free' label from fcoe_init function
On 06/02/2017 11:19 AM, Julia Lawall wrote: > > > On Fri, 2 Jun 2017, Milan P. Gandhi wrote: > >> On 06/01/2017 08:32 PM, Dan Carpenter wrote: >>> On Thu, Jun 01, 2017 at 05:41:06PM +0530, Milan P. Gandhi wrote: Simplify the check for return code of fcoe_if_init routine in fcoe_init function such that we could eliminate need for extra 'out_free' label. Signed-off-by: Milan P. Gandhi--- drivers/scsi/fcoe/fcoe.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index ea21e7b..fb2a4c9 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c @@ -2523,13 +2523,11 @@ static int __init fcoe_init(void) fcoe_dev_setup(); rc = fcoe_if_init(); - if (rc) - goto out_free; - - mutex_unlock(_config_mutex); - return 0; + if (rc == 0) { + mutex_unlock(_config_mutex); + return 0; + } -out_free: mutex_unlock(_config_mutex); >>> >>> Gar... Stop! No1 Don't do this. >>> >>> Do failure handling, not success handling. >>> >>> People always think they should get creative with the last if statement >>> in a function. This leads to spaghetti code and it's confusing. Please >>> never do this again. >>> >>> The original is correct and the new code is bad rubbish code. >>> >>> regards, >>> dan carpenter >>> >>> >> >> Oops, my bad sir. Will keep this in mind. > > Still, does the mutex_unlock really need to be duplicated? > > julia > Hello Julia, Thanks for your hint! I have found a better way to remove a need for duplicate mutex_unlock statement and extra 'out_free' label. Will send the corrected path for the same. Many thanks, Milan.
[PATCH] qla2xxx: remove writeq/readq function definitions
Instead of rewriting write/readq, use linux/io-64-nonatomic-lo-hi.h which already have them. Signed-off-by: Corentin Labbe--- drivers/scsi/qla2xxx/qla_nx.h | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_nx.h b/drivers/scsi/qla2xxx/qla_nx.h index 77624eac95a4..71a41093530e 100644 --- a/drivers/scsi/qla2xxx/qla_nx.h +++ b/drivers/scsi/qla2xxx/qla_nx.h @@ -7,6 +7,8 @@ #ifndef __QLA_NX_H #define __QLA_NX_H +#include + /* * Following are the states of the Phantom. Phantom will set them and * Host will read to check if the fields are correct. @@ -819,21 +821,6 @@ struct qla82xx_uri_data_desc{ #define MIU_TEST_AGT_WRDATA_UPPER_LO (0x0b0) #defineMIU_TEST_AGT_WRDATA_UPPER_HI(0x0b4) -#ifndef readq -static inline u64 readq(void __iomem *addr) -{ - return readl(addr) | (((u64) readl(addr + 4)) << 32LL); -} -#endif - -#ifndef writeq -static inline void writeq(u64 val, void __iomem *addr) -{ - writel(((u32) (val)), (addr)); - writel(((u32) (val >> 32)), (addr + 4)); -} -#endif - /* Request and response queue size */ #define REQUEST_ENTRY_CNT_82XX 128 /* Number of request entries. */ #define RESPONSE_ENTRY_CNT_82XX128 /* Number of response entries.*/ -- 2.13.0
Re: [PATCH v2 12/12] xen/scsifront: Remove code that zeroes driver-private command data
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCH v2 11/12] virtio: Remove code that zeroes driver-private command data
Nit: the driver name is virtio_scsi, not just virtio. Otherwise this looks fine: Reviewed-by: Christoph Hellwig
Re: [PATCH v2 10/12] snic: Remove code that zeroes driver-private command data
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCH v2 09/12] Make scsi_mq_prep_fn() call scsi_init_command()
I like this idea, but.. > -void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) > +void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd, > +struct scsi_data_buffer *prot_sdb) > { > void *buf = cmd->sense_buffer; > - void *prot = cmd->prot_sdb; > unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA; > > /* zero out the cmd, except for the embedded scsi_request */ > @@ -1164,7 +1164,7 @@ void scsi_init_command(struct scsi_device *dev, struct > scsi_cmnd *cmd) > > cmd->device = dev; > cmd->sense_buffer = buf; > - cmd->prot_sdb = prot; > + cmd->prot_sdb = prot_sdb; What would be the problem of always preserving the original prot_sdb value instead of passing it by argument? You;d just need to initialize it in scsi_init_request when the command is allocated.
Re: [PATCH v2 08/12] Introduce scsi_mq_sgl_size()
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCH v2 06/12] Make __scsi_remove_device go straight from BLOCKED to DEL
On Thu, Jun 01, 2017 at 04:27:05PM -0700, Bart Van Assche wrote: > If a device is blocked, make __scsi_remove_device() cause it to > transition to the DEL state. This means that all the commands > issued in .shutdown() will error in the mid-layer, thus making > the removal proceed without being stopped. > > This patch is a slightly modified version of a patch from James > Bottomley. This patch avoids that the following lockup occurs: Do we really need the printk for this case? And if we do we should probably rate limit it.
Re: [PATCH v2 05/12] Introduce scsi_start_queue()
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCH v2 03/12] Create two versions of scsi_internal_device_unblock()
Looks fine, Reviewed-by: Christoph Hellwig
Re: [PATCH v2 04/12] Protect SCSI device state changes with a mutex
On Thu, Jun 01, 2017 at 04:27:03PM -0700, Bart Van Assche wrote: > Enable this mechanism for all scsi_target_*block() callers but not > for the scsi_internal_device_unblock() calls from the mpt3sas driver > because that driver can call scsi_internal_device_unblock() from > atomic context. This is missing an explanation on why we'd want to serialize them using a mutex.
Re: [PATCH v2 02/12] Split scsi_internal_device_block()
Yes, much better: Reviewed-by: Christoph Hellwig
Re: [PATCH v2 01/12] Avoid that scsi_exit_rq() triggers a use-after-free
Looks fine, Reviewed-by: Christoph Hellwig
[PATCH net-next 4/4] qed: Add support for changing iSCSI mac
Enhance API between qedi and qed, allowing qedi to inform device's firmware when the iSCSI mac is to be changed. Signed-off-by: Yuval Mintz--- drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 66 + drivers/net/ethernet/qlogic/qed/qed_sp.h| 1 + include/linux/qed/qed_iscsi_if.h| 7 +++ 3 files changed, 74 insertions(+) diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c index bc8ce09..787f66e 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c +++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c @@ -488,6 +488,54 @@ static int qed_sp_iscsi_conn_update(struct qed_hwfn *p_hwfn, return qed_spq_post(p_hwfn, p_ent, NULL); } +static int +qed_sp_iscsi_mac_update(struct qed_hwfn *p_hwfn, + struct qed_iscsi_conn *p_conn, + enum spq_mode comp_mode, + struct qed_spq_comp_cb *p_comp_addr) +{ + struct iscsi_spe_conn_mac_update *p_ramrod = NULL; + struct qed_spq_entry *p_ent = NULL; + struct qed_sp_init_data init_data; + int rc = -EINVAL; + u8 ucval; + + /* Get SPQ entry */ + memset(_data, 0, sizeof(init_data)); + init_data.cid = p_conn->icid; + init_data.opaque_fid = p_hwfn->hw_info.opaque_fid; + init_data.comp_mode = comp_mode; + init_data.p_comp_data = p_comp_addr; + + rc = qed_sp_init_request(p_hwfn, _ent, +ISCSI_RAMROD_CMD_ID_MAC_UPDATE, +PROTOCOLID_ISCSI, _data); + if (rc) + return rc; + + p_ramrod = _ent->ramrod.iscsi_conn_mac_update; + p_ramrod->hdr.op_code = ISCSI_RAMROD_CMD_ID_MAC_UPDATE; + SET_FIELD(p_ramrod->hdr.flags, + ISCSI_SLOW_PATH_HDR_LAYER_CODE, p_conn->layer_code); + + p_ramrod->conn_id = cpu_to_le16(p_conn->conn_id); + p_ramrod->fw_cid = cpu_to_le32(p_conn->icid); + ucval = p_conn->remote_mac[1]; + ((u8 *)(_ramrod->remote_mac_addr_hi))[0] = ucval; + ucval = p_conn->remote_mac[0]; + ((u8 *)(_ramrod->remote_mac_addr_hi))[1] = ucval; + ucval = p_conn->remote_mac[3]; + ((u8 *)(_ramrod->remote_mac_addr_mid))[0] = ucval; + ucval = p_conn->remote_mac[2]; + ((u8 *)(_ramrod->remote_mac_addr_mid))[1] = ucval; + ucval = p_conn->remote_mac[5]; + ((u8 *)(_ramrod->remote_mac_addr_lo))[0] = ucval; + ucval = p_conn->remote_mac[4]; + ((u8 *)(_ramrod->remote_mac_addr_lo))[1] = ucval; + + return qed_spq_post(p_hwfn, p_ent, NULL); +} + static int qed_sp_iscsi_conn_terminate(struct qed_hwfn *p_hwfn, struct qed_iscsi_conn *p_conn, enum spq_mode comp_mode, @@ -1324,6 +1372,23 @@ static int qed_iscsi_stats(struct qed_dev *cdev, struct qed_iscsi_stats *stats) return qed_iscsi_get_stats(QED_LEADING_HWFN(cdev), stats); } +static int qed_iscsi_change_mac(struct qed_dev *cdev, + u32 handle, const u8 *mac) +{ + struct qed_hash_iscsi_con *hash_con; + + hash_con = qed_iscsi_get_hash(cdev, handle); + if (!hash_con) { + DP_NOTICE(cdev, "Failed to find connection for handle %d\n", + handle); + return -EINVAL; + } + + return qed_sp_iscsi_mac_update(QED_LEADING_HWFN(cdev), + hash_con->con, + QED_SPQ_MODE_EBLOCK, NULL); +} + void qed_get_protocol_stats_iscsi(struct qed_dev *cdev, struct qed_mcp_iscsi_stats *stats) { @@ -1358,6 +1423,7 @@ static const struct qed_iscsi_ops qed_iscsi_ops_pass = { .destroy_conn = _iscsi_destroy_conn, .clear_sq = _iscsi_clear_conn_sq, .get_stats = _iscsi_stats, + .change_mac = _iscsi_change_mac, }; const struct qed_iscsi_ops *qed_get_iscsi_ops(void) diff --git a/drivers/net/ethernet/qlogic/qed/qed_sp.h b/drivers/net/ethernet/qlogic/qed/qed_sp.h index b9464f3..00dd50f 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_sp.h +++ b/drivers/net/ethernet/qlogic/qed/qed_sp.h @@ -120,6 +120,7 @@ union ramrod_data { struct iscsi_spe_func_dstry iscsi_destroy; struct iscsi_spe_conn_offload iscsi_conn_offload; struct iscsi_conn_update_ramrod_params iscsi_conn_update; + struct iscsi_spe_conn_mac_update iscsi_conn_mac_update; struct iscsi_spe_conn_termination iscsi_conn_terminate; struct vf_start_ramrod_data vf_start; diff --git a/include/linux/qed/qed_iscsi_if.h b/include/linux/qed/qed_iscsi_if.h index 3414649..111e606 100644 --- a/include/linux/qed/qed_iscsi_if.h +++ b/include/linux/qed/qed_iscsi_if.h @@ -210,6 +210,11 @@ struct qed_iscsi_cb_ops { * @param stats - pointer to struck that would be filled * we
[PATCH net-next 3/4] qed: Support NVM-image reading API
Storage drivers require images from the nvram in boot-from-SAN scenarios. This provides the necessary API between qed and the protocol drivers to perform such reads. Signed-off-by: Yuval Mintz--- drivers/net/ethernet/qlogic/qed/qed_main.c | 16 ++ drivers/net/ethernet/qlogic/qed/qed_mcp.c | 89 ++ drivers/net/ethernet/qlogic/qed/qed_mcp.h | 21 +++ include/linux/qed/qed_if.h | 18 ++ 4 files changed, 144 insertions(+) diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index baebd59..6ac10ce 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -1535,6 +1535,21 @@ static int qed_drain(struct qed_dev *cdev) return 0; } +static int qed_nvm_get_image(struct qed_dev *cdev, enum qed_nvm_images type, +u8 *buf, u16 len) +{ + struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev); + struct qed_ptt *ptt = qed_ptt_acquire(hwfn); + int rc; + + if (!ptt) + return -EAGAIN; + + rc = qed_mcp_get_nvm_image(hwfn, ptt, type, buf, len); + qed_ptt_release(hwfn, ptt); + return rc; +} + static void qed_get_coalesce(struct qed_dev *cdev, u16 *rx_coal, u16 *tx_coal) { *rx_coal = cdev->rx_coalesce_usecs; @@ -1712,6 +1727,7 @@ const struct qed_common_ops qed_common_ops_pass = { .dbg_all_data_size = _dbg_all_data_size, .chain_alloc = _chain_alloc, .chain_free = _chain_free, + .nvm_get_image = _nvm_get_image, .get_coalesce = _get_coalesce, .set_coalesce = _set_coalesce, .set_led = _set_led, diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c index e82f329..9da9104 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c @@ -2310,6 +2310,95 @@ int qed_mcp_bist_nvm_test_get_image_att(struct qed_hwfn *p_hwfn, return rc; } +static int +qed_mcp_get_nvm_image_att(struct qed_hwfn *p_hwfn, + struct qed_ptt *p_ptt, + enum qed_nvm_images image_id, + struct qed_nvm_image_att *p_image_att) +{ + struct bist_nvm_image_att mfw_image_att; + enum nvm_image_type type; + u32 num_images, i; + int rc; + + /* Translate image_id into MFW definitions */ + switch (image_id) { + case QED_NVM_IMAGE_ISCSI_CFG: + type = NVM_TYPE_ISCSI_CFG; + break; + case QED_NVM_IMAGE_FCOE_CFG: + type = NVM_TYPE_FCOE_CFG; + break; + default: + DP_NOTICE(p_hwfn, "Unknown request of image_id %08x\n", + image_id); + return -EINVAL; + } + + /* Learn number of images, then traverse and see if one fits */ + rc = qed_mcp_bist_nvm_test_get_num_images(p_hwfn, p_ptt, _images); + if (rc || !num_images) + return -EINVAL; + + for (i = 0; i < num_images; i++) { + rc = qed_mcp_bist_nvm_test_get_image_att(p_hwfn, p_ptt, +_image_att, i); + if (rc) + return rc; + + if (type == mfw_image_att.image_type) + break; + } + if (i == num_images) { + DP_VERBOSE(p_hwfn, QED_MSG_STORAGE, + "Failed to find nvram image of type %08x\n", + image_id); + return -EINVAL; + } + + p_image_att->start_addr = mfw_image_att.nvm_start_addr; + p_image_att->length = mfw_image_att.len; + + return 0; +} + +int qed_mcp_get_nvm_image(struct qed_hwfn *p_hwfn, + struct qed_ptt *p_ptt, + enum qed_nvm_images image_id, + u8 *p_buffer, u32 buffer_len) +{ + struct qed_nvm_image_att image_att; + int rc; + + memset(p_buffer, 0, buffer_len); + + rc = qed_mcp_get_nvm_image_att(p_hwfn, p_ptt, image_id, _att); + if (rc) + return rc; + + /* Validate sizes - both the image's and the supplied buffer's */ + if (image_att.length <= 4) { + DP_VERBOSE(p_hwfn, QED_MSG_STORAGE, + "Image [%d] is too small - only %d bytes\n", + image_id, image_att.length); + return -EINVAL; + } + + /* Each NVM image is suffixed by CRC; Upper-layer has no need for it */ + image_att.length -= 4; + + if (image_att.length > buffer_len) { + DP_VERBOSE(p_hwfn, + QED_MSG_STORAGE, + "Image [%d] is too big - %08x bytes where only %08x are available\n", + image_id, image_att.length, buffer_len); +