Re: blk-merge: BIO front merge size check for chunked devices
On 02/02/2016 08:06 AM, Damien Le Moal wrote: > > Hello, > > In ll_front_merge_fn, the request size result of the eventual front > merge of a BIO with a request is checked against the maximum > size allowed for the request using blk_rq_get_max_sectors. This > function will also check that the merge result will not span block > chunks for chunked block devices using the function blk_max_size_offset. > > However, blk_rq_get_max_sectors always calls blk_max_size_offset using > the request sector position, which in the case of a front merge is not > the position at which the merged request would be issued: the sector > position to use must be that of the BIO, and not that of the request. > > This problem can trigger a “boundary violation error” for write > requests on ZAC/ZBC host-managed SMR disks as the last write BIO > of a zone (a chunk) can end up being front-merged with the first > request of the following zone (chunk). > > The attached patch against linux-4.5-rc2 fixes this problem by adding > an offset argument to the function blk_rq_get_max_sectors. > Reviewed-by: Hannes Reinecke I can easily respin this as a proper patch if required. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
blk-merge: BIO front merge size check for chunked devices
Hello, In ll_front_merge_fn, the request size result of the eventual front merge of a BIO with a request is checked against the maximum size allowed for the request using blk_rq_get_max_sectors. This function will also check that the merge result will not span block chunks for chunked block devices using the function blk_max_size_offset. However, blk_rq_get_max_sectors always calls blk_max_size_offset using the request sector position, which in the case of a front merge is not the position at which the merged request would be issued: the sector position to use must be that of the BIO, and not that of the request. This problem can trigger a “boundary violation error” for write requests on ZAC/ZBC host-managed SMR disks as the last write BIO of a zone (a chunk) can end up being front-merged with the first request of the following zone (chunk). The attached patch against linux-4.5-rc2 fixes this problem by adding an offset argument to the function blk_rq_get_max_sectors. Best regards. diff -Naur linux-4.5-rc2/block/blk-merge.c linux-4.5-rc2-patched/block/blk-merge.c --- linux-4.5-rc2/block/blk-merge.c 2016-02-01 11:12:16.0 +0900 +++ linux-4.5-rc2-patched/block/blk-merge.c 2016-02-02 15:28:07.626107509 +0900 @@ -504,7 +504,7 @@ integrity_req_gap_back_merge(req, bio)) return 0; if (blk_rq_sectors(req) + bio_sectors(bio) > - blk_rq_get_max_sectors(req)) { + blk_rq_get_max_sectors(req, blk_rq_pos(req))) { req->cmd_flags |= REQ_NOMERGE; if (req == q->last_merge) q->last_merge = NULL; @@ -528,7 +528,7 @@ integrity_req_gap_front_merge(req, bio)) return 0; if (blk_rq_sectors(req) + bio_sectors(bio) > - blk_rq_get_max_sectors(req)) { + blk_rq_get_max_sectors(req, bio->bi_iter.bi_sector)) { req->cmd_flags |= REQ_NOMERGE; if (req == q->last_merge) q->last_merge = NULL; @@ -574,7 +574,7 @@ * Will it become too large? */ if ((blk_rq_sectors(req) + blk_rq_sectors(next)) > - blk_rq_get_max_sectors(req)) + blk_rq_get_max_sectors(req, blk_rq_pos(req))) return 0; total_phys_segments = req->nr_phys_segments + next->nr_phys_segments; diff -Naur linux-4.5-rc2/include/linux/blkdev.h linux-4.5-rc2-patched/include/linux/blkdev.h --- linux-4.5-rc2/include/linux/blkdev.h2016-02-01 11:12:16.0 +0900 +++ linux-4.5-rc2-patched/include/linux/blkdev.h2016-02-02 15:26:39.313283902 +0900 @@ -888,7 +888,8 @@ (offset & (q->limits.chunk_sectors - 1)); } -static inline unsigned int blk_rq_get_max_sectors(struct request *rq) +static inline unsigned int blk_rq_get_max_sectors(struct request *rq, + sector_t offset) { struct request_queue *q = rq->q; @@ -898,7 +899,7 @@ if (!q->limits.chunk_sectors || (rq->cmd_flags & REQ_DISCARD)) return blk_queue_get_max_sectors(q, rq->cmd_flags); - return min(blk_max_size_offset(q, blk_rq_pos(rq)), + return min(blk_max_size_offset(q, offset), blk_queue_get_max_sectors(q, rq->cmd_flags)); } -- Damien Le Moal, System Software Group, WW Research, HGST, a Western Digital company Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer: This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited. If you have received this e-mail in error, please notify the sender immediately and delete the e-mail in its entirety from your system. bio-merge-size-check.patch Description: bio-merge-size-check.patch
Re: [PATCH 2/2] iscsi_ibft: Always display netmask
On 01/22/2016 01:49 PM, Lee Duncan wrote: > From: Hannes Reinecke > > Some older user-space code might rely on the netmask attribute > being present, so we should always display it. > This fixes a regression introduced by commit > 0b2eb3c4060a16f3ec11a4d6d4c934e7e5d5334f. > > Signed-off-by: Hannes Reinecke > Signed-off-by: Lee Duncan > --- > drivers/firmware/iscsi_ibft.c | 8 +--- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c > index 2dd1fbb8..81037e5fe301 100644 > --- a/drivers/firmware/iscsi_ibft.c > +++ b/drivers/firmware/iscsi_ibft.c > @@ -464,14 +464,8 @@ static umode_t ibft_check_nic_for(void *data, int type) > rc = S_IRUGO; > break; > case ISCSI_BOOT_ETH_PREFIX_LEN: > - if (nic->subnet_mask_prefix) > - rc = S_IRUGO; > - break; > case ISCSI_BOOT_ETH_SUBNET_MASK: > - if (!memcmp(nic->ip_addr, nulls, 10) && > - (nic->ip_addr[10] == 0xff) && > - (nic->ip_addr[11] == 0xff) && > - nic->subnet_mask_prefix) > + if (nic->subnet_mask_prefix) > rc = S_IRUGO; > break; > case ISCSI_BOOT_ETH_ORIGIN: > Sorry. I thought I sent this mail already. Is the commit id above supposed to be referencing the first patch? I could not match it to anything. If so, then shouldn't this patch just be combined with the second patch and some comment about us always displaying it for compat reasons added to the code? Also, you should normally cc Konrad for iscsi_ibft.c patches, because he is actually the maintainer. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Separate target visibility from reaped state information
On 01/19/16 17:03, James Bottomley wrote: On Tue, 2016-01-19 at 19:30 -0500, Martin K. Petersen wrote: "Bart" == Bart Van Assche writes: Bart> Instead of representing the states "visible in sysfs" and "has Bart> been removed from the target list" by a single state variable, use Bart> two variables to represent this information. James: Are you happy with the latest iteration of this? Should I queue it? Well, I'm OK with the patch: it's a simple transformation of the enumerated state to a two bit state. What I can't see is how it fixes any soft lockup. The only change from the current workflow is that the DEL transition (now the reaped flag) is done before the spin lock is dropped which would fix a tiny window for two threads both trying to remove the same target, but there's nothing that could possibly fix an iterative soft lockup caused by restarting the loop, which is what the changelog says. Hello James, scsi_remove_target() doesn't lock the scan_mutex which means that concurrent SCSI scanning activity is not prohibited. Such scanning activity can postpone the transition of the state of a SCSI target into STARGET_DEL. I think if the scheduler decides to run the thread that executes scsi_remove_target() on the same CPU as the scanning code after the scanning code has obtained a reap ref and before the scanning code has released the reap ref again that the soft lockup can be triggered that has been reported by Sebastian Herbszt. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 00/23] ALUA device handler update, part II
> "Hannes" == Hannes Reinecke writes: Hannes> as promised here is now the second part of my ALUA device Hannes> handler update. This contains a major rework of the ALUA device Hannes> handler as execution is moved onto a workqueue. This has the Hannes> advantage that we avoid having to do multiple calls to the same Hannes> LUN (as happens frequently when failing over a LUN with several Hannes> paths) and finally retries are handled correctly. I would really like to get this series merged but I need reviews for: [PATCHv4 13/23] scsi_dh_alua: Use workqueue for RTPG https://patchwork.kernel.org/patch/8058991/ [PATCHv4 15/23] scsi_dh_alua: Add new blacklist flag 'BLIST_SYNC_ALUA' https://patchwork.kernel.org/patch/8058951/ [PATCHv4 22/23] scsi_dh_alua: update 'access_state' field https://patchwork.kernel.org/patch/8059021/ -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v2 00/23] HiSilicon SAS v2 hw support
> "John" == John Garry writes: John> This patchset introduces support for the HiSi SAS v2 hw. The John> major difference between v1 and v2 hw is support for SATA/STP. Applied to 4.6/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] megaraid: fix null pointer check in megasas_detach_one().
> "Maurizio" == Maurizio Lombardi writes: Maurizio> The pd_seq_sync pointer can't be NULL, we have to check its Maurizio> entries instead. Sumit? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: bnx2fc_eh_abort(): fix wrong return code.
> "Maurizio" == Maurizio Lombardi writes: Maurizio> If the link is not ready, the bnx2fc_eh_abort() function Maurizio> should return FAILED. Applied to 4.6/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: Show information about log levels in 'modinfo'
> "Jose" == Jose Castillo writes: Jose> This patch adds the information of the different values that can Jose> be used in the module parameter 'debug_logging', as it is shown Jose> below: Applied to 4.6/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/12] be2iscsi: Remove unused mcc_cq_lock
> "James" == James Bottomley writes: James> On Mon, 2016-02-01 at 15:42 +0530, Jitendra Bhivare wrote: >> mcc_cq_lock spin_lock is used only in beiscsi_process_mcc which is >> called only when all interrupts are disabled from mgmt_epfw_cleanup >> during unloading of driver. There is no other context where there can >> be contention for the processing of CQ. James> Removing a lock is not a bug fix unless it's causing a user James> visible problem, so this patch (and quite a lot of others in this James> series) should go through the merge window process. James> For things that cause user visible problems, we need a James> description of the problem in the changelog and a cc to stable James> unless it was a regression in the 4.4+ merge window. I applied be2iscsi 11.0.0.0 to 4.6/scsi-queue about a week ago so this is inevitably merge window material. Jitendra, it's a good idea to spell out the intended target release or repository when you post patches. Makes it easier for everyone. Instead of "[PATCH 00/12] be2iscsi: critical fixes for 11.0.0.0" I would suggest something like "[PATCH 00/12] be2iscsi: important fixes for 11.0.0.0 driver in 4.6/scsi-queue". That way we know which tree to apply the patches to and it's clear that it's an incremental update to a previously queued series. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it
> "Hannes" == Hannes Reinecke writes: Hannes> And in anycase, I guess we should be using the same logic sd.c Hannes> is using. Please see the attached patch. I'm OK with this change but please send it as a proper patch submission so somebody else can review it. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052
Hello Martin, > -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Monday, February 1, 2016 4:57 PM > To: Raghava Aditya Renukunta > Cc: james.bottom...@hansenpartnership.com; > martin.peter...@oracle.com; linux-scsi@vger.kernel.org; Mahesh > Rajashekhara; Murthy Bhat; Gana Sridaran; aacr...@pmc-sierra.com; Scott > Benesh; jthumsh...@suse.de; the...@redhat.com; > shane.seym...@hpe.com > Subject: Re: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052 > > > "Raghava" == Raghava Aditya Renukunta > writes: > > Raghava, > > Raghava> This patchset includes the following changes (bug fixes and new > Raghava> feature support) specific to aacraid driver. > > Please fix the build failure in patch 3/9 when CONFIG_PM is not defined > and address Tomas' comments for 7/9. I will create a new patch version fixing these issues shortly. Thank you Martin. Regards, Raghava Aditya > -- > Martin K. PetersenOracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
Hello Tomas, > -Original Message- > From: Tomas Henzl [mailto:the...@redhat.com] > Sent: Monday, February 1, 2016 8:44 AM > To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com; > martin.peter...@oracle.com; linux-scsi@vger.kernel.org > Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc- > sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com > Subject: Re: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET > > On 29.1.2016 23:45, Raghava Aditya Renukunta wrote: > > From: Raghava Aditya Renukunta > > > > while driver removal is in progress or PCI shutdown is invoked, driver > > kills AIF aacraid thread, but IOCTL requests from the management tools > > re-start AIF thread leading to IOP_RESET. > > > > Fixed by setting adapter_shutdown flag when PCI shutdown is invoked. > > > > Changes in V2: > > Set adapter_shutdown flag before shutdown command is sent to \ > > controller > > > > Changes in V3: > > Call aac_send_shut_shutdown first thing in __aac_shutdown > > Convert adapter_shutdown to atomic_t variable to prevent \ > > SMP coherency issues(race conditions) > > > > Changes in V4: > > Used mutex to protect ioctl path and adapter_shutdown to prevent \ > > race conditions. > > > > Signed-off-by: Raghava Aditya Renukunta > > > Reviewed-by: Shane Seymour > > Reviewed-by: Johannes Thumshirn > > --- > > drivers/scsi/aacraid/aacraid.h | 2 +- > > drivers/scsi/aacraid/commctrl.c | 3 ++ > > drivers/scsi/aacraid/comminit.c | 6 ++-- > > drivers/scsi/aacraid/linit.c| 63 +++-- > > > 4 files changed, 56 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h > > index 2916288..6c55749 100644 > > --- a/drivers/scsi/aacraid/aacraid.h > > +++ b/drivers/scsi/aacraid/aacraid.h > > @@ -1123,7 +1123,7 @@ struct aac_dev > > > > struct fib *free_fib; > > spinlock_t fib_lock; > > - > > + struct mutexioctl_mutex; > > struct aac_queue_block *queues; > > /* > > * The user API will use an IOCTL to register itself to receive > > diff --git a/drivers/scsi/aacraid/commctrl.c > b/drivers/scsi/aacraid/commctrl.c > > index 54195a1..8d3438c 100644 > > --- a/drivers/scsi/aacraid/commctrl.c > > +++ b/drivers/scsi/aacraid/commctrl.c > > @@ -855,6 +855,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void > __user *arg) > > { > > int status; > > > > + if (dev->adapter_shutdown) > > + return -EACCES; > > + > > /* > > * HBA gets first crack > > */ > > diff --git a/drivers/scsi/aacraid/comminit.c > b/drivers/scsi/aacraid/comminit.c > > index 0e954e3..2b4e753 100644 > > --- a/drivers/scsi/aacraid/comminit.c > > +++ b/drivers/scsi/aacraid/comminit.c > > @@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev) > > return -ENOMEM; > > aac_fib_init(fibctx); > > > > - cmd = (struct aac_close *) fib_data(fibctx); > > + mutex_lock(&dev->ioctl_mutex); > > + dev->adapter_shutdown = 1; > > + mutex_unlock(&dev->ioctl_mutex); > > > > + cmd = (struct aac_close *) fib_data(fibctx); > > cmd->command = cpu_to_le32(VM_CloseAll); > > cmd->cid = cpu_to_le32(0xfffe); > > > > @@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev) > > /* FIB should be freed only after getting the response from the F/W > */ > > if (status != -ERESTARTSYS) > > aac_fib_free(fibctx); > > - dev->adapter_shutdown = 1; > > if ((dev->pdev->device == PMC_DEVICE_S7 || > > dev->pdev->device == PMC_DEVICE_S8 || > > dev->pdev->device == PMC_DEVICE_S9) && > > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > > index af8974e..9453e11 100644 > > --- a/drivers/scsi/aacraid/linit.c > > +++ b/drivers/scsi/aacraid/linit.c > > @@ -524,10 +524,17 @@ static struct device_attribute *aac_dev_attrs[] = { > > > > static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg) > > { > > - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata; > > + int ret; > > + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata; > > + > > if (!capable(CAP_SYS_RAWIO)) > > return -EPERM; > > - return aac_do_ioctl(dev, cmd, arg); > > + > > + mutex_lock(&aac->ioctl_mutex); > > + ret = aac_do_ioctl(aac, cmd, arg); > > + mutex_unlock(&aac->ioctl_mutex); > > + > > + return ret; > > } > > > > static int aac_eh_abort(struct scsi_cmnd* cmd) > > @@ -704,13 +711,14 @@ static long aac_cfg_ioctl(struct file *file, > > unsigned int cmd, unsigned long arg) > > { > > int ret; > > - struct aac_dev *aac; > > - aac = (struct aac_dev *)file->private_data; > > - if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown) > > + struct aac_dev *aac = (struct aac_dev *)file->private_data; > > + > > + if (!capable(CAP_SYS_RAWIO)) > > return -EPERM; > > - mutex_lock(&aa
Re: [PATCH 0/2] RESEND: Enable iBFT IPv6 booting with prefix length
> "Lee" == Lee Duncan writes: Lee> [Resending. Apologies if you get this twice.] It turns out that Lee> IPv6 doesn't care about netmask, but instead uses "prefix length" Lee> to determine the subnet, but the iBFT subsystem still just supports Lee> netmask. Mike, care to review? https://patchwork.kernel.org/patch/8092511/ https://patchwork.kernel.org/patch/8092501/ -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Separate target visibility from reaped state information
> "Sebastian" == Sebastian Herbszt writes: >> The only change from the current workflow is that the DEL transition >> (now the reaped flag) is done before the spin lock is dropped which >> would fix a tiny window for two threads both trying to remove the >> same target, but there's nothing that could possibly fix an iterative >> soft lockup caused by restarting the loop, which is what the >> changelog says. Sebastian> James, Martin, what's the status of this patch? I still hit Sebastian> the reported soft lockup on 4.5-rc1. And you have verified that Bart's patch applied on top of 4.5-rc1 still fixes the lockup? (I know you tested a previous version) I am concerned about queuing something as a stable fix if it is just masking a fundamental underlying problem. James' comment suggests that there is something else going on. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 0/9] aacraid: Patchset for aacraid driver version 41052
> "Raghava" == Raghava Aditya Renukunta > writes: Raghava, Raghava> This patchset includes the following changes (bug fixes and new Raghava> feature support) specific to aacraid driver. Please fix the build failure in patch 3/9 when CONFIG_PM is not defined and address Tomas' comments for 7/9. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add support for DWC UFS Host Controller
Hi Joao, On Mon, Feb 1, 2016 at 11:47 PM, Joao Pinto wrote: > This patch includes: > - quirks in the ufs core driver to support Synopsys MPHY Test Chip config > - quirks in the ufs core driver to support DWC configuration sequence > - New Unipro attributes were added > - ufs core driver was tweaked to support UFS 2.0 > - support for Synopsys PCI ID in the pci glue driver > - new platform glue driver for Synopsys devices > > Signed-off-by: Joao Pinto > --- > Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 16 + > drivers/scsi/ufs/Kconfig | 54 ++ > drivers/scsi/ufs/Makefile | 1 + > drivers/scsi/ufs/ufs-dwc.c| 115 +++ > drivers/scsi/ufs/ufshcd-pci.c | 2 + > drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +- > drivers/scsi/ufs/ufshcd.c | 822 > +- > drivers/scsi/ufs/ufshcd.h | 15 + > drivers/scsi/ufs/ufshci.h | 29 + > drivers/scsi/ufs/unipro.h | 39 + > 10 files changed, 1089 insertions(+), 6 deletions(-) > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt > create mode 100644 drivers/scsi/ufs/ufs-dwc.c You should separate out your changes into separate patches, e.g. 1. Fix the spelling mistake 2. Update the base code for UFSHCI_VERSION_20 compatiblity. 3. Implement any common code as a separate module 4. Add the OF and PCI drivers > diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > new file mode 100644 > index 000..fa361f2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > @@ -0,0 +1,16 @@ > +* Universal Flash Storage (UFS) DesignWare Host Controller > + > +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers. > +Each UFS controller instance should have its own node. > + > +Required properties: > +- compatible: compatible list, contains "snps,ufshcd" > +- reg : > +- interrupts: > + > +Example: > + dwc_ufshcd@0xD000 { > + compatible = "snps,ufshcd"; > + reg = < 0xD000 0x1 >; > + interrupts = < 24 >; > + }; If I recall correctly, when adding device tree bindings, you need to cc the devicetree list, devicet...@vger.kernel.org. > diff --git a/drivers/scsi/ufs/ufs-dwc.c b/drivers/scsi/ufs/ufs-dwc.c > new file mode 100644 > index 000..e4d70b7 > --- /dev/null > +++ b/drivers/scsi/ufs/ufs-dwc.c > @@ -0,0 +1,115 @@ > +/* == > + * The Synopsys DWC UFS Software Driver and documentation (hereinafter > + * "Software") is an unsupported proprietary work of Synopsys, Inc. unless > + * otherwise expressly agreed to in writing between Synopsys and you. > + * > + * The Software IS NOT an item of Licensed Software or Licensed Product under > + * any End User Software License Agreement or Agreement for Licensed Product > + * with Synopsys or any supplement thereto. Permission is hereby granted, > + * free of charge, to any person obtaining a copy of this software annotated > + * with this license and the Software, to deal in the Software without > + * restriction, including without limitation the rights to use, copy, modify, > + * merge, publish, distribute, sublicense, and/or sell copies of the > Software, > + * and to permit persons to whom the Software is furnished to do so, subject > + * to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THIS SOFTWARE IS BEING DISTRIBUTED BY SYNOPSYS SOLELY ON AN "AS IS" BASIS > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE HEREBY DISCLAIMED. IN NO EVENT SHALL SYNOPSYS BE LIABLE FOR ANY > DIRECT, > + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES > + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR > + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER > + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > SUCH > + * DAMAGE. > + * == Is this GPLv2 compatible? > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include "ufshcd.h" > +#include "ufshcd-pltfrm.h" > + > +/** > + * struct ufs_hba_dwc_vops - UFS DWC specific variant operations > + * > + */ > +static struct ufs_hba_variant_ops ufs_hba_dwc_vops = { > + .name = "dw
investici
dobrý vecer, Rád bych Vás informovali o investici stejných výhod pro nás, a ty me meli kontaktovat na e-mailu níe tomto, pokud máte zájem. email: fng.ni...@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] scsi_transport_fc: LUN masking
> "Hannes" == Hannes Reinecke writes: Hannes> Hmm. this seemed to have fallen through -ENOREVIEWS So, yes. Please resend. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 PATCH 1/3] scsi:stex.c Support to Pegasus series.
> "Charles" == Charles Chiou writes: Charles> Does this patch has others issues need to fix? Please resubmit your patch series with the review tag from Johannes added. Thank you! -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
Hi Kai, > -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of "Kai Mäkisara (Kolumbus)" > Sent: Tuesday, February 02, 2016 5:43 AM > To: Seymour, Shane M > Cc: Laurence Oberman; Emmanuel Florac; Laurence Oberman; linux- > s...@vger.kernel.org > Subject: Re: What partition should the MTMKPART argument specify? Was: > Re: st driver doesn't seem to grok LTO partitioning > > > > On 1.2.2016, at 8.31, Seymour, Shane M > wrote: > > > > Hi Kai, > > > > Thanks for the changes the HPE DAT72 DDS5 drive now works as expected: > > > Good. Thanks for testing. > > ... > > > > I'm asking around again one final time to see if I can lay my hands on a > > LTO5 > or greater drive so I can test LTO partitioning as well. > > > > The only other thing I can think of (I'm not sure if this is an improvement > > or > not) is if bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + > PP_OFF_NBR_ADD_PARTS] (max.parts and nbr_parts in the debug message) > is zero just return -EINVAL unless you know of any take drives that report > them both as 0 but can be partitioned? That is after this: > > > >DEBC_printk(STp, "psd_cnt %d, max.parts %d, nbr_parts %d\n", > >psd_cnt, bp[pgo + PP_OFF_MAX_ADD_PARTS], > >bp[pgo + PP_OFF_NBR_ADD_PARTS]); > > > > add (and also turn off the can-partitions option): > > > > if ((bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + > PP_OFF_NBR_ADD_PARTS]) == 0) { > > DEBC_printk(STp, "Drive not partitionable - > max.parts+nbr_parts is 0\n"); > > STp->can_partitions = 0; > > return -EINVAL; > > } > > > > I'm not especially fussed if you don't want to add that though. > > > I thought about a test like this (only test maximum number) but decided not > to add it. The reason was that I did not want to change anything that has > worked before. I quite trust that the current drives return sense data instead > of crashing and the end result for the user would be the same. However, one > can argue that returning EINVAL is better than EIO but does the user notice? > If the common opinion is that a test like this should be added, I am not > against it. It can be added to the code for SCSI >=3 where it does not risk > anything for the old drives. > > IMHO, can_partitions should not be cleared based on the test. For example, > trying to partition a LTO-4 tape in a LTO-5 drive should not disable > partitioning. > (The mode page should return zero as maximum number of partitions when > a LTO-4 tape is inserted.) No problem, I didn't think of the case where you have a non-partitionable tape in a drive that can do partitions. That case should have been obvious to me. I may be able to lay my hands on a LTO5+ drive (only a small chance). Someone in the US is checking is checking for me and will hook it up to the system I use for testing tape stuff for me. I'll only have it for about a week if I'm able to get it. Thanks Shane > > Thanks, > Kai > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] megaraid_sas: add an i/o barrier
On Mon, Feb 01, 2016 at 03:12:04PM +0100, Tomas Henzl wrote: > A barrier should be added to ensure proper > ordering of memory mapped writes. > > V2: - added the barrier also to megasas_fire_cmd_skinny, > as suggested by Kashyap Desai > > Signed-off-by: Tomas Henzl > --- > drivers/scsi/megaraid/megaraid_sas_base.c | 1 + > drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + > 2 files changed, 2 insertions(+) This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read Documentation/stable_kernel_rules.txt for how to do this properly. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
The new patch did not work for me, but I chatted with Shane and I have his mt version. I will update my DAT to same firmware or newer than his and provide a second tested by. I also expect my LTO5 to show up this week so will be ready for that. Thanks everyone for keeping tapes alive Laurence Oberman Principal Software Maintenance Engineer Red Hat Global Support Services - Original Message - From: "Kai Mäkisara (Kolumbus)" To: "Shane M Seymour" Cc: "Laurence Oberman" , "Emmanuel Florac" , "Laurence Oberman" , linux-scsi@vger.kernel.org Sent: Monday, February 1, 2016 1:43:26 PM Subject: Re: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning > On 1.2.2016, at 8.31, Seymour, Shane M wrote: > > Hi Kai, > > Thanks for the changes the HPE DAT72 DDS5 drive now works as expected: > Good. Thanks for testing. ... > > I'm asking around again one final time to see if I can lay my hands on a LTO5 > or greater drive so I can test LTO partitioning as well. > > The only other thing I can think of (I'm not sure if this is an improvement > or not) is if bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + PP_OFF_NBR_ADD_PARTS] > (max.parts and nbr_parts in the debug message) is zero just return -EINVAL > unless you know of any take drives that report them both as 0 but can be > partitioned? That is after this: > >DEBC_printk(STp, "psd_cnt %d, max.parts %d, nbr_parts %d\n", >psd_cnt, bp[pgo + PP_OFF_MAX_ADD_PARTS], >bp[pgo + PP_OFF_NBR_ADD_PARTS]); > > add (and also turn off the can-partitions option): > > if ((bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + PP_OFF_NBR_ADD_PARTS]) > == 0) { > DEBC_printk(STp, "Drive not partitionable - max.parts+nbr_parts > is 0\n"); > STp->can_partitions = 0; > return -EINVAL; > } > > I'm not especially fussed if you don't want to add that though. > I thought about a test like this (only test maximum number) but decided not to add it. The reason was that I did not want to change anything that has worked before. I quite trust that the current drives return sense data instead of crashing and the end result for the user would be the same. However, one can argue that returning EINVAL is better than EIO but does the user notice? If the common opinion is that a test like this should be added, I am not against it. It can be added to the code for SCSI >=3 where it does not risk anything for the old drives. IMHO, can_partitions should not be cleared based on the test. For example, trying to partition a LTO-4 tape in a LTO-5 drive should not disable partitioning. (The mode page should return zero as maximum number of partitions when a LTO-4 tape is inserted.) Thanks, Kai -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning
> On 1.2.2016, at 8.31, Seymour, Shane M wrote: > > Hi Kai, > > Thanks for the changes the HPE DAT72 DDS5 drive now works as expected: > Good. Thanks for testing. ... > > I'm asking around again one final time to see if I can lay my hands on a LTO5 > or greater drive so I can test LTO partitioning as well. > > The only other thing I can think of (I'm not sure if this is an improvement > or not) is if bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + PP_OFF_NBR_ADD_PARTS] > (max.parts and nbr_parts in the debug message) is zero just return -EINVAL > unless you know of any take drives that report them both as 0 but can be > partitioned? That is after this: > >DEBC_printk(STp, "psd_cnt %d, max.parts %d, nbr_parts %d\n", >psd_cnt, bp[pgo + PP_OFF_MAX_ADD_PARTS], >bp[pgo + PP_OFF_NBR_ADD_PARTS]); > > add (and also turn off the can-partitions option): > > if ((bp[pgo + PP_OFF_MAX_ADD_PARTS] + bp[pgo + PP_OFF_NBR_ADD_PARTS]) > == 0) { > DEBC_printk(STp, "Drive not partitionable - max.parts+nbr_parts > is 0\n"); > STp->can_partitions = 0; > return -EINVAL; > } > > I'm not especially fussed if you don't want to add that though. > I thought about a test like this (only test maximum number) but decided not to add it. The reason was that I did not want to change anything that has worked before. I quite trust that the current drives return sense data instead of crashing and the end result for the user would be the same. However, one can argue that returning EINVAL is better than EIO but does the user notice? If the common opinion is that a test like this should be added, I am not against it. It can be added to the code for SCSI >=3 where it does not risk anything for the old drives. IMHO, can_partitions should not be cleared based on the test. For example, trying to partition a LTO-4 tape in a LTO-5 drive should not disable partitioning. (The mode page should return zero as maximum number of partitions when a LTO-4 tape is inserted.) Thanks, Kai -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 111441] New: iscsi fails to attach to targets
On 01/30/2016 01:38 AM, Nicholas A. Bellinger wrote: > On Fri, 2016-01-29 at 17:32 -0600, Mike Christie wrote: >> On 01/29/2016 04:21 PM, Serguei Bezverkhi (sbezverk) wrote: >>> HI Mike, >>> >>> I tried your patch and it is has eliminated first traceback but I still do >>> not see my remote targets. >>> >> >> That is sort of expected. Your target is not setup for ALUA properly. It >> says it supports ALUA, but when scsi_dh_alua asks about the ports it is >> reporting there are none. Ccing the people that made the patch that >> added the issue and own the code. >> >> Hey Christoph and Hannes, >> >> The dh/alua changes that added this: >> >> error = scsi_dh_add_device(sdev); >> if (error) { >> sdev_printk(KERN_INFO, sdev, >> "failed to add device handler: %d\n", >> error); >> return error; >> } >> >> to scsi_sysfs_add_sdev are adding a regression. >> >> 1. If that fails, then we forget to do device_del before doing the >> return. My patch in this thread added that back, so we do not see the >> sysfs oopses anymore. But. >> >> 2. It looks like in older kernels, we would allow misconfigured targets >> like this one to still setup devices. Do we want that old behavior back? >> Should we just ignore the return value from scsi_dh_add_device above? >> Note that in this case, it is LIO so it can be easily fixed on the >> target side by just setting it up properly. I do not think other targets >> would hit this type of issue. >> > > Btw, what does misconfigured mean here wrt target ALUA..? [ 25.833195] sd 6:0:0:4: alua: supports implicit and explicit TPGS [ 25.833360] sd 6:0:0:4: alua: No target port descriptors found [ 25.833363] sd 6:0:0:4: alua: Attach failed (-22) [ 25.833365] sd 6:0:0:4: failed to add device handler: -22 He has LIO configured to report it supports implicit/explicit ALUA, but the ports do not seem to be configured. For the LIO config side, are his LUNs just not in a the default_lu_gp or any other group? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 7/9] aacraid: Fix AIF triggered IOP_RESET
On 29.1.2016 23:45, Raghava Aditya Renukunta wrote: > From: Raghava Aditya Renukunta > > while driver removal is in progress or PCI shutdown is invoked, driver > kills AIF aacraid thread, but IOCTL requests from the management tools > re-start AIF thread leading to IOP_RESET. > > Fixed by setting adapter_shutdown flag when PCI shutdown is invoked. > > Changes in V2: > Set adapter_shutdown flag before shutdown command is sent to \ > controller > > Changes in V3: > Call aac_send_shut_shutdown first thing in __aac_shutdown > Convert adapter_shutdown to atomic_t variable to prevent \ > SMP coherency issues(race conditions) > > Changes in V4: > Used mutex to protect ioctl path and adapter_shutdown to prevent \ > race conditions. > > Signed-off-by: Raghava Aditya Renukunta > Reviewed-by: Shane Seymour > Reviewed-by: Johannes Thumshirn > --- > drivers/scsi/aacraid/aacraid.h | 2 +- > drivers/scsi/aacraid/commctrl.c | 3 ++ > drivers/scsi/aacraid/comminit.c | 6 ++-- > drivers/scsi/aacraid/linit.c| 63 > +++-- > 4 files changed, 56 insertions(+), 18 deletions(-) > > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h > index 2916288..6c55749 100644 > --- a/drivers/scsi/aacraid/aacraid.h > +++ b/drivers/scsi/aacraid/aacraid.h > @@ -1123,7 +1123,7 @@ struct aac_dev > > struct fib *free_fib; > spinlock_t fib_lock; > - > + struct mutexioctl_mutex; > struct aac_queue_block *queues; > /* >* The user API will use an IOCTL to register itself to receive > diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c > index 54195a1..8d3438c 100644 > --- a/drivers/scsi/aacraid/commctrl.c > +++ b/drivers/scsi/aacraid/commctrl.c > @@ -855,6 +855,9 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void > __user *arg) > { > int status; > > + if (dev->adapter_shutdown) > + return -EACCES; > + > /* >* HBA gets first crack >*/ > diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c > index 0e954e3..2b4e753 100644 > --- a/drivers/scsi/aacraid/comminit.c > +++ b/drivers/scsi/aacraid/comminit.c > @@ -212,8 +212,11 @@ int aac_send_shutdown(struct aac_dev * dev) > return -ENOMEM; > aac_fib_init(fibctx); > > - cmd = (struct aac_close *) fib_data(fibctx); > + mutex_lock(&dev->ioctl_mutex); > + dev->adapter_shutdown = 1; > + mutex_unlock(&dev->ioctl_mutex); > > + cmd = (struct aac_close *) fib_data(fibctx); > cmd->command = cpu_to_le32(VM_CloseAll); > cmd->cid = cpu_to_le32(0xfffe); > > @@ -229,7 +232,6 @@ int aac_send_shutdown(struct aac_dev * dev) > /* FIB should be freed only after getting the response from the F/W */ > if (status != -ERESTARTSYS) > aac_fib_free(fibctx); > - dev->adapter_shutdown = 1; > if ((dev->pdev->device == PMC_DEVICE_S7 || >dev->pdev->device == PMC_DEVICE_S8 || >dev->pdev->device == PMC_DEVICE_S9) && > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c > index af8974e..9453e11 100644 > --- a/drivers/scsi/aacraid/linit.c > +++ b/drivers/scsi/aacraid/linit.c > @@ -524,10 +524,17 @@ static struct device_attribute *aac_dev_attrs[] = { > > static int aac_ioctl(struct scsi_device *sdev, int cmd, void __user * arg) > { > - struct aac_dev *dev = (struct aac_dev *)sdev->host->hostdata; > + int ret; > + struct aac_dev *aac = (struct aac_dev *)sdev->host->hostdata; > + > if (!capable(CAP_SYS_RAWIO)) > return -EPERM; > - return aac_do_ioctl(dev, cmd, arg); > + > + mutex_lock(&aac->ioctl_mutex); > + ret = aac_do_ioctl(aac, cmd, arg); > + mutex_unlock(&aac->ioctl_mutex); > + > + return ret; > } > > static int aac_eh_abort(struct scsi_cmnd* cmd) > @@ -704,13 +711,14 @@ static long aac_cfg_ioctl(struct file *file, > unsigned int cmd, unsigned long arg) > { > int ret; > - struct aac_dev *aac; > - aac = (struct aac_dev *)file->private_data; > - if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown) > + struct aac_dev *aac = (struct aac_dev *)file->private_data; > + > + if (!capable(CAP_SYS_RAWIO)) > return -EPERM; > - mutex_lock(&aac_mutex); > - ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg); > - mutex_unlock(&aac_mutex); > + > + mutex_lock(&aac->ioctl_mutex); > + ret = aac_do_ioctl(aac, cmd, (void __user *)arg); > + mutex_unlock(&aac->ioctl_mutex); > > return ret; > } > @@ -719,7 +727,10 @@ static long aac_cfg_ioctl(struct file *file, > static long aac_compat_do_ioctl(struct aac_dev *dev, unsigned cmd, unsigned > long arg) > { > long ret; > - mutex_lock(&aac_mutex); > + > + if (dev->adapter_shutdown) > + return -EACCES; There is another test for the s
[PATCH] target: fix cast from pointer to phys_addr_t
The uio_mem structure has a member that is a phys_addr_t, but can be a number of other types too. The target core driver attempts to assign a pointer from vmalloc() to it, by casting it to phys_addr_t, but that causes a warning when phys_addr_t is longer than a pointer: drivers/target/target_core_user.c: In function 'tcmu_configure_device': drivers/target/target_core_user.c:906:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] This adds another cast to uintptr_t to shut up the warning. A nicer fix might be to have additional fields in uio_mem for the different purposes, so we can assign a pointer directly. Signed-off-by: Arnd Bergmann --- This has been around for a long while, but did not trigger in my randconfig testing until now, don't know why. diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index dd600e5ead71..94f5154ac788 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -903,7 +903,7 @@ static int tcmu_configure_device(struct se_device *dev) info->version = __stringify(TCMU_MAILBOX_VERSION); info->mem[0].name = "tcm-user command & data buffer"; - info->mem[0].addr = (phys_addr_t) udev->mb_addr; + info->mem[0].addr = (phys_addr_t)(uintptr_t)udev->mb_addr; info->mem[0].size = TCMU_RING_SIZE; info->mem[0].memtype = UIO_MEM_VIRTUAL; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: bnx2fc_eh_abort(): fix wrong return code.
On Mon, 1 Feb 2016, Maurizio Lombardi wrote: If the link is not ready, the bnx2fc_eh_abort() function should return FAILED. Signed-off-by: Maurizio Lombardi --- drivers/scsi/bnx2fc/bnx2fc_io.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c index 0002caf..2230dab 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_io.c +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c @@ -1104,8 +1104,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) struct bnx2fc_cmd *io_req; struct fc_lport *lport; struct bnx2fc_rport *tgt; - int rc = FAILED; - + int rc; rc = fc_block_scsi_eh(sc_cmd); if (rc) @@ -1114,7 +1113,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) lport = shost_priv(sc_cmd->device->host); if ((lport->state != LPORT_ST_READY) || !(lport->link_up)) { printk(KERN_ERR PFX "eh_abort: link not ready\n"); - return rc; + return FAILED; } tgt = (struct bnx2fc_rport *)&rp[1]; Yes, nice catch. Acked-by: Chad Dupuis -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bnx2fc: bnx2fc_eh_abort(): fix wrong return code.
On Mon, 2016-02-01 at 16:08 +0100, Maurizio Lombardi wrote: > If the link is not ready, the bnx2fc_eh_abort() function > should return FAILED. > > Signed-off-by: Maurizio Lombardi > --- > drivers/scsi/bnx2fc/bnx2fc_io.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c > index 0002caf..2230dab 100644 > --- a/drivers/scsi/bnx2fc/bnx2fc_io.c > +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c > @@ -1104,8 +1104,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) > struct bnx2fc_cmd *io_req; > struct fc_lport *lport; > struct bnx2fc_rport *tgt; > - int rc = FAILED; > - > + int rc; > > rc = fc_block_scsi_eh(sc_cmd); > if (rc) > @@ -1114,7 +1113,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) > lport = shost_priv(sc_cmd->device->host); > if ((lport->state != LPORT_ST_READY) || !(lport->link_up)) { > printk(KERN_ERR PFX "eh_abort: link not ready\n"); > - return rc; > + return FAILED; > } > > tgt = (struct bnx2fc_rport *)&rp[1]; Yes, because setting the initial setting of rc = FAILED is overwritten by the result of fc_block_scsi_eh(). Reviewed-by: Ewan D. Milne -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] bnx2fc: bnx2fc_eh_abort(): fix wrong return code.
If the link is not ready, the bnx2fc_eh_abort() function should return FAILED. Signed-off-by: Maurizio Lombardi --- drivers/scsi/bnx2fc/bnx2fc_io.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c index 0002caf..2230dab 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_io.c +++ b/drivers/scsi/bnx2fc/bnx2fc_io.c @@ -1104,8 +1104,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) struct bnx2fc_cmd *io_req; struct fc_lport *lport; struct bnx2fc_rport *tgt; - int rc = FAILED; - + int rc; rc = fc_block_scsi_eh(sc_cmd); if (rc) @@ -1114,7 +1113,7 @@ int bnx2fc_eh_abort(struct scsi_cmnd *sc_cmd) lport = shost_priv(sc_cmd->device->host); if ((lport->state != LPORT_ST_READY) || !(lport->link_up)) { printk(KERN_ERR PFX "eh_abort: link not ready\n"); - return rc; + return FAILED; } tgt = (struct bnx2fc_rport *)&rp[1]; -- Maurizio Lombardi -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] megaraid_sas: add an i/o barrier
A barrier should be added to ensure proper ordering of memory mapped writes. V2: - added the barrier also to megasas_fire_cmd_skinny, as suggested by Kashyap Desai Signed-off-by: Tomas Henzl --- drivers/scsi/megaraid/megaraid_sas_base.c | 1 + drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index cc92c8198d..9f7689515c 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -749,6 +749,7 @@ megasas_fire_cmd_skinny(struct megasas_instance *instance, &(regs)->inbound_high_queue_port); writel((lower_32_bits(frame_phys_addr) | (frame_count<<1))|1, &(regs)->inbound_low_queue_port); + mmiowb(); spin_unlock_irqrestore(&instance->hba_lock, flags); } diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index d9d0029fb1..98a848bdfd 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct megasas_instance *instance, &instance->reg_set->inbound_low_queue_port); writel(le32_to_cpu(req_desc->u.high), &instance->reg_set->inbound_high_queue_port); + mmiowb(); spin_unlock_irqrestore(&instance->hba_lock, flags); #endif } -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: megaraid_sas: add an i/o barrier
> -Original Message- > From: Tomas Henzl [mailto:the...@redhat.com] > Sent: Monday, February 01, 2016 6:23 PM > To: Kashyap Desai; linux-scsi@vger.kernel.org > Cc: Sumit Saxena; Uday Lingala > Subject: Re: megaraid_sas: add an i/o barrier > > On 1.2.2016 05:45, Kashyap Desai wrote: > >> -Original Message- > >> From: Tomas Henzl [mailto:the...@redhat.com] > >> Sent: Friday, January 29, 2016 11:05 PM > >> To: 'linux-scsi@vger.kernel.org' > >> Cc: sumit.sax...@avagotech.com; Desai, Kashyap; Uday Lingala > >> Subject: megaraid_sas: add an i/o barrier > >> > >> A barrier should be added to ensure proper ordering of memory mapped > >> writes. > >> > >> Signed-off-by: Tomas Henzl > >> --- > >> drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c > >> b/drivers/scsi/megaraid/megaraid_sas_fusion.c > >> index d9d0029fb1..98a848bdfd 100644 > >> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c > >> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c > >> @@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct > megasas_instance > >> *instance, > >>&instance->reg_set->inbound_low_queue_port); > >>writel(le32_to_cpu(req_desc->u.high), > >>&instance->reg_set->inbound_high_queue_port); > >> + mmiowb(); > >>spin_unlock_irqrestore(&instance->hba_lock, flags); #endif } > > Tomas- > > > > We may need similar changes around below Functions as well, because > > there is no associated readX or mmiowb() call. > > megasas_fire_cmd_xscale() > > megasas_fire_cmd_ppc() > > megasas_fire_cmd_skinny() > > megasas_fire_cmd_gen2() > > > > Also, wrireq() routine in same function megasas_fire_cmd_fusion() > > need i/o barrier. > > I don't think so (with the exception of megasas_fire_cmd_skinny - I missed > this one). > When two threads try to use a fire_cmd there is no protection of certain > ordering, that had to be done in a caller of fire_cmd (for example in > megasas_build_and_issue_cmd_fusion) > and it seems to me that there is nothing like that. Likely is, that this - > a > strict ordering of commands - is not needed. > The protection which I'm adding is needed when a command consist of a > sequence of more than one write, see memory-barriers.txt. > > (It looks to me that > megasas_fire_cmd_ -xscale -ppc -skiny -gen2 do not need the hba_lock > unless there is another i/o sequence protected with the same lock (note > also that there is no such lock in megasas_fire_cmd_fusion).) Agree, we don't need hba lock in older fire_cmd wrapper. We updated fusion code because it was active shipment and product life cycle was active for fusion adapter, so code changes was tested by Avago. We did not clean code in MFI based controller since it is only in critical support cycle. > > > I'll add the mmiowb to megasas_fire_cmd_skinny and send a new patch - > agreed? Got your word. You are right. Yes additional changes in megasas_fire_cmd_skinny along with current patch will be good to go. > > --tms > > > > >> -- > >> 2.4.3 > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/14] Big fixes, retries, handle a race condition
V7: updated patch 0001 according to a comment also, removed patch 07/15 from V6, so now there are only 14 patches V6: update Reviewed-by from various reviewers V5: removed un-necessary wmb() V4: fixing a few comments from reviewers V3: removed specific calls to wmb() since they are redundant. V2: a few minor changes V1: This serie of 15 small patches should be pushed after the series of 8 patches I have uploaded to the upstream a week ago: "Fix error message and present UFS variant probe" Yaniv Gardi (14): scsi: ufs: clear UTRD, UPIU req and rsp before new transfers scsi: ufs: clear fields UTRD, UPIU req and rsp before new transfers scsi: ufs: verify command tag validity scsi: ufs: clear outstanding_request bit in case query timeout scsi: ufs: increase fDeviceInit query response timeout scsi: ufs: avoid exception event handler racing with PM callbacks scsi: ufs: add retries to dme_peer get and set attribute scsi: ufs: add retries for hibern8 enter scsi: ufs: fix error recovery after the hibern8 exit failure scsi: ufs: retry failed query flag requests scsi: ufs: reduce the interrupts for power mode change requests scsi: ufs: add missing memory barriers scsi: ufs: commit descriptors before setting the doorbell scsi: ufs: add wrapper for retrying sending query attribute drivers/scsi/ufs/ufshcd.c | 395 -- drivers/scsi/ufs/ufshcd.h | 4 + 2 files changed, 319 insertions(+), 80 deletions(-) -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 02/14] scsi: ufs: clear fields UTRD, UPIU req and rsp before new transfers
Some of the data structures (like response UPIU) and/or its elements (unused fields) should be cleared before sending out the respective command to UFS device. This change clears the UPIU response data structure for query commands and NOP command before sending out the command. We also initialize the PRDT table length to zero which should take care of commands which doesn't have any data associated with it. We are also clearing the unused fields in request UPIU for NOP command. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 03533f0..7ae87c9 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1129,6 +1129,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp, cpu_to_le32(OCS_INVALID_COMMAND_STATUS); /* dword_3 is reserved, hence it is set to 0 */ req_desc->header.dword_3 = 0; + + req_desc->prd_table_length = 0; } /** @@ -1197,6 +1199,7 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba, if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC) memcpy(descp, query->descriptor, len); + memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp)); } static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp) @@ -1209,6 +1212,11 @@ static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp) ucd_req_ptr->header.dword_0 = UPIU_HEADER_DWORD( UPIU_TRANSACTION_NOP_OUT, 0, 0, lrbp->task_tag); + /* clear rest of the fields of basic header */ + ucd_req_ptr->header.dword_1 = 0; + ucd_req_ptr->header.dword_2 = 0; + + memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp)); } /** -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 04/14] scsi: ufs: clear outstanding_request bit in case query timeout
When sending a query to the device returns with a timeout error, we clear the corresponding bit in the DOORBELL register but we don't clear the outstanding_request field as we should. This patch fixes this bug. Reviewed-by: Subhash Jadavani Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8a34f61..4863e93 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -364,6 +364,16 @@ static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) } /** + * ufshcd_outstanding_req_clear - Clear a bit in outstanding request field + * @hba: per adapter instance + * @tag: position of the bit to be cleared + */ +static inline void ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag) +{ + __clear_bit(tag, &hba->outstanding_reqs); +} + +/** * ufshcd_get_lists_status - Check UCRDY, UTRLRDY and UTMRLRDY * @reg: Register value of host controller status * @@ -1501,9 +1511,17 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba, if (!time_left) { err = -ETIMEDOUT; + dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n", + __func__, lrbp->task_tag); if (!ufshcd_clear_cmd(hba, lrbp->task_tag)) - /* sucessfully cleared the command, retry if needed */ + /* successfully cleared the command, retry if needed */ err = -EAGAIN; + /* +* in case of an error, after clearing the doorbell, +* we also need to clear the outstanding_request +* field in hba +*/ + ufshcd_outstanding_req_clear(hba, lrbp->task_tag); } return err; @@ -3941,7 +3959,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) scsi_dma_unmap(cmd); spin_lock_irqsave(host->host_lock, flags); - __clear_bit(tag, &hba->outstanding_reqs); + ufshcd_outstanding_req_clear(hba, tag); hba->lrb[tag].cmd = NULL; spin_unlock_irqrestore(host->host_lock, flags); -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/14] scsi: ufs: verify command tag validity
A race condition appear to exist between request completion when scsi_done() is called to end the request and set the tag back to -1 (at blk_queue_end_tag() scsi_end_request), and scsi layer error handling which aborts the command and reuses it to request sense data. Sending the request sense is done with tag which was set to -1 and so it is invalid. Assert command tag passed from scsi layer is valid. Reviewed-by: Subhash Jadavani Reviewed-by: Dolev Raviv Signed-off-by: Gilad Broner Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 24 ++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 7ae87c9..8a34f61 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -190,6 +190,10 @@ static int ufshcd_config_pwr_mode(struct ufs_hba *hba, struct ufs_pa_layer_attr *desired_pwr_mode); static int ufshcd_change_power_mode(struct ufs_hba *hba, struct ufs_pa_layer_attr *pwr_mode); +static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag) +{ + return tag >= 0 && tag < hba->nutrs; +} static inline int ufshcd_enable_irq(struct ufs_hba *hba) { @@ -1309,6 +1313,12 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) hba = shost_priv(host); tag = cmd->request->tag; + if (!ufshcd_valid_tag(hba, tag)) { + dev_err(hba->dev, + "%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p", + __func__, tag, cmd, cmd->request); + BUG(); + } spin_lock_irqsave(hba->host->host_lock, flags); switch (hba->ufshcd_state) { @@ -3861,13 +3871,23 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) host = cmd->device->host; hba = shost_priv(host); tag = cmd->request->tag; + if (!ufshcd_valid_tag(hba, tag)) { + dev_err(hba->dev, + "%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p", + __func__, tag, cmd, cmd->request); + BUG(); + } ufshcd_hold(hba, false); + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); /* If command is already aborted/completed, return SUCCESS */ - if (!(test_bit(tag, &hba->outstanding_reqs))) + if (!(test_bit(tag, &hba->outstanding_reqs))) { + dev_err(hba->dev, + "%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n", + __func__, tag, hba->outstanding_reqs, reg); goto out; + } - reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); if (!(reg & (1 << tag))) { dev_err(hba->dev, "%s: cmd was completed, but without a notifying intr, tag = %d", -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 05/14] scsi: ufs: increase fDeviceInit query response timeout
fDeviceInit query response time for some devices is too long that default query request timeout of 100ms may not be enough. Experiments show that fDeviceInit response sometimes takes 500ms so to be on safer side this change sets the timeout to 600ms. Without this change, we might unnecessarily have to retry fDeviceInit query requests multiple times and each query request timeout prints one error message. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 4863e93..e74c53a 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -58,6 +58,12 @@ #define QUERY_REQ_RETRIES 10 /* Query request timeout */ #define QUERY_REQ_TIMEOUT 30 /* msec */ +/* + * Query request timeout for fDeviceInit flag + * fDeviceInit query response time for some devices is too large that default + * QUERY_REQ_TIMEOUT may not be enough for such devices. + */ +#define QUERY_FDEVICEINIT_REQ_TIMEOUT 600 /* msec */ /* Task management command timeout */ #define TM_CMD_TIMEOUT 100 /* msecs */ @@ -1650,6 +1656,7 @@ static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode, struct ufs_query_req *request = NULL; struct ufs_query_res *response = NULL; int err, index = 0, selector = 0; + int timeout = QUERY_REQ_TIMEOUT; BUG_ON(!hba); @@ -1682,7 +1689,10 @@ static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode, goto out_unlock; } - err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, QUERY_REQ_TIMEOUT); + if (idn == QUERY_FLAG_IDN_FDEVICEINIT) + timeout = QUERY_FDEVICEINIT_REQ_TIMEOUT; + + err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_QUERY, timeout); if (err) { dev_err(hba->dev, -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 09/14] scsi: ufs: fix error recovery after the hibern8 exit failure
Hibern8 exit can be called from 3 different context: - ufshcd_hibern8_exit_work - ufshcd_ungate_work - runtime/system resume If hibern8 exit fails for some reason then we try to bring the link to active state by link startup but this recovery mechanism results into deadlock or errors from first 2 context listed above. This change fixes the recovery by adding proper error handling mechanism. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 58 +++ 1 file changed, 53 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 1581d03..da08a40 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -608,6 +608,11 @@ int ufshcd_hold(struct ufs_hba *hba, bool async) spin_lock_irqsave(hba->host->host_lock, flags); hba->clk_gating.active_reqs++; + if (ufshcd_eh_in_progress(hba)) { + spin_unlock_irqrestore(hba->host->host_lock, flags); + return 0; + } + start: switch (hba->clk_gating.state) { case CLKS_ON: @@ -723,7 +728,8 @@ static void __ufshcd_release(struct ufs_hba *hba) if (hba->clk_gating.active_reqs || hba->clk_gating.is_suspended || hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL || hba->lrb_in_use || hba->outstanding_tasks - || hba->active_uic_cmd || hba->uic_async_done) + || hba->active_uic_cmd || hba->uic_async_done + || ufshcd_eh_in_progress(hba)) return; hba->clk_gating.state = REQ_CLKS_OFF; @@ -1360,6 +1366,13 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) cmd->scsi_done(cmd); goto out_unlock; } + + /* if error handling is in progress, don't issue commands */ + if (ufshcd_eh_in_progress(hba)) { + set_host_byte(cmd, DID_ERROR); + cmd->scsi_done(cmd); + goto out_unlock; + } spin_unlock_irqrestore(hba->host->host_lock, flags); /* acquire the tag to make sure device cmds don't use it */ @@ -2390,6 +2403,31 @@ out: return ret; } +static int ufshcd_link_recovery(struct ufs_hba *hba) +{ + int ret; + unsigned long flags; + + spin_lock_irqsave(hba->host->host_lock, flags); + hba->ufshcd_state = UFSHCD_STATE_RESET; + ufshcd_set_eh_in_progress(hba); + spin_unlock_irqrestore(hba->host->host_lock, flags); + + ret = ufshcd_host_reset_and_restore(hba); + + spin_lock_irqsave(hba->host->host_lock, flags); + if (ret) + hba->ufshcd_state = UFSHCD_STATE_ERROR; + ufshcd_clear_eh_in_progress(hba); + spin_unlock_irqrestore(hba->host->host_lock, flags); + + if (ret) + dev_err(hba->dev, "%s: link recovery failed, err %d", + __func__, ret); + + return ret; +} + static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba) { int ret; @@ -2398,10 +2436,18 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba) uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); - if (ret) + if (ret) { dev_err(hba->dev, "%s: hibern8 enter failed. ret = %d\n", __func__, ret); + /* +* If link recovery fails then return error so that caller +* don't retry the hibern8 enter again. +*/ + if (ufshcd_link_recovery(hba)) + ret = -ENOLINK; + } + return ret; } @@ -2426,8 +2472,9 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); if (ret) { - ufshcd_set_link_off(hba); - ret = ufshcd_host_reset_and_restore(hba); + dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n", + __func__, ret); + ret = ufshcd_link_recovery(hba); } return ret; @@ -4379,7 +4426,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba) /* UFS device is also active now */ ufshcd_set_ufs_dev_active(hba); ufshcd_force_reset_auto_bkops(hba); - hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL; hba->wlun_dev_clr_ua = true; if (ufshcd_get_max_pwr_mode(hba)) { @@ -4393,6 +4439,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba) __func__, ret); } + /* set the state as operational after switching to desired gear */ + hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL; /* * If we are in error handling context or in power management callbacks * context, no n
[PATCH v7 07/14] scsi: ufs: add retries to dme_peer get and set attribute
The dme_peer get/set attribute commands are prone to errors, therefore we add three retries for the UIC command sending. Error code returned from ufshcd_send_uic_cmd() is checked, and unless it was successful or the retries have finished, another command will be sent. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Lee Susman Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 40 +--- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e374b79..4c0827f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -68,6 +68,9 @@ /* Task management command timeout */ #define TM_CMD_TIMEOUT 100 /* msecs */ +/* maximum number of retries for a general UIC command */ +#define UFS_UIC_COMMAND_RETRIES 3 + /* maximum number of link-startup retries */ #define DME_LINKSTARTUP_RETRIES 3 @@ -2182,6 +2185,7 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, }; const char *set = action[!!peer]; int ret; + int retries = UFS_UIC_COMMAND_RETRIES; uic_cmd.command = peer ? UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET; @@ -2189,10 +2193,18 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel, uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set); uic_cmd.argument3 = mib_val; - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); - if (ret) - dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n", - set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret); + do { + /* for peer attributes we retry upon failure */ + ret = ufshcd_send_uic_cmd(hba, &uic_cmd); + if (ret) + dev_dbg(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n", + set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret); + } while (ret && peer && --retries); + + if (!retries) + dev_err(hba->dev, "%s: attr-id 0x%x val 0x%x failed %d retries\n", + set, UIC_GET_ATTR_ID(attr_sel), mib_val, + retries); return ret; } @@ -2217,6 +2229,7 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, }; const char *get = action[!!peer]; int ret; + int retries = UFS_UIC_COMMAND_RETRIES; struct ufs_pa_layer_attr orig_pwr_info; struct ufs_pa_layer_attr temp_pwr_info; bool pwr_mode_change = false; @@ -2247,14 +2260,19 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel, UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET; uic_cmd.argument1 = attr_sel; - ret = ufshcd_send_uic_cmd(hba, &uic_cmd); - if (ret) { - dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n", - get, UIC_GET_ATTR_ID(attr_sel), ret); - goto out; - } + do { + /* for peer attributes we retry upon failure */ + ret = ufshcd_send_uic_cmd(hba, &uic_cmd); + if (ret) + dev_dbg(hba->dev, "%s: attr-id 0x%x error code %d\n", + get, UIC_GET_ATTR_ID(attr_sel), ret); + } while (ret && peer && --retries); + + if (!retries) + dev_err(hba->dev, "%s: attr-id 0x%x failed %d retries\n", + get, UIC_GET_ATTR_ID(attr_sel), retries); - if (mib_val) + if (mib_val && !ret) *mib_val = uic_cmd.argument3; if (peer && (hba->quirks & UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE) -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 06/14] scsi: ufs: avoid exception event handler racing with PM callbacks
If device raises the exception event in the response to the commands sent during the runtime/system PM callbacks, exception event handler might run in parallel with PM callbacks and may see unclocked register accesses. This change fixes this issue by not scheduling the exception event handler while PM callbacks are running. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e74c53a..e374b79 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3144,7 +3144,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) scsi_status = result & MASK_SCSI_STATUS; result = ufshcd_scsi_cmd_status(lrbp, scsi_status); - if (ufshcd_is_exception_event(lrbp->ucd_rsp_ptr)) + /* +* Currently we are only supporting BKOPs exception +* events hence we can ignore BKOPs exception event +* during power management callbacks. BKOPs exception +* event is not expected to be raised in runtime suspend +* callback as it allows the urgent bkops. +* During system suspend, we are anyway forcefully +* disabling the bkops and if urgent bkops is needed +* it will be enabled on system resume. Long term +* solution could be to abort the system suspend if +* UFS device needs urgent BKOPs. +*/ + if (!hba->pm_op_in_progress && + ufshcd_is_exception_event(lrbp->ucd_rsp_ptr)) schedule_work(&hba->eeh_work); break; case UPIU_TRANSACTION_REJECT_UPIU: -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 13/14] scsi: ufs: commit descriptors before setting the doorbell
Add a write memory barrier to make sure descriptors prepared are actually written to memory before ringing the doorbell. We have also added the write memory barrier after ringing the doorbell register so that controller sees the new request immediately. Reviewed-by: Dolev Raviv Signed-off-by: Gilad Broner Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 1893a14..2fc678d 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1625,6 +1625,8 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, hba->dev_cmd.complete = &wait; + /* Make sure descriptors are ready before ringing the doorbell */ + wmb(); spin_lock_irqsave(hba->host->host_lock, flags); ufshcd_send_command(hba, tag); spin_unlock_irqrestore(hba->host->host_lock, flags); -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 08/14] scsi: ufs: add retries for hibern8 enter
If hibern8 enter command fails then UFS link state may be unknown which may result into timeout of all the commands issued after failure. This change does 2 things (for pre-defined number of retry counts) after hibern8 enter failure: 1. Recovers the UFS link to active state 2. If link is recovered to active state, tries to put the UFS link in hibern8 enter again until retry count expires. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 4c0827f..1581d03 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -74,6 +74,9 @@ /* maximum number of link-startup retries */ #define DME_LINKSTARTUP_RETRIES 3 +/* Maximum retries for Hibern8 enter */ +#define UIC_HIBERN8_ENTER_RETRIES 3 + /* maximum number of reset retries before giving up */ #define MAX_HOST_RESET_RETRIES 5 @@ -2387,13 +2390,32 @@ out: return ret; } -static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) +static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba) { + int ret; struct uic_command uic_cmd = {0}; uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; + ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd); + + if (ret) + dev_err(hba->dev, "%s: hibern8 enter failed. ret = %d\n", + __func__, ret); + + return ret; +} + +static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) +{ + int ret = 0, retries; - return ufshcd_uic_pwr_ctrl(hba, &uic_cmd); + for (retries = UIC_HIBERN8_ENTER_RETRIES; retries > 0; retries--) { + ret = __ufshcd_uic_hibern8_enter(hba); + if (!ret || ret == -ENOLINK) + goto out; + } +out: + return ret; } static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 10/14] scsi: ufs: retry failed query flag requests
UFS flag query requests may fail sometimes due to timeouts etc. Add a wrapper function to retry up to 10 times in case of such failure, similar to retries being made for attribute queries. Reviewed-by: Dolev Raviv Signed-off-by: Gilad Broner Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 63 --- drivers/scsi/ufs/ufshcd.h | 4 +++ 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index da08a40..87da429 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1660,6 +1660,29 @@ static inline void ufshcd_init_query(struct ufs_hba *hba, (*request)->upiu_req.selector = selector; } +static int ufshcd_query_flag_retry(struct ufs_hba *hba, + enum query_opcode opcode, enum flag_idn idn, bool *flag_res) +{ + int ret; + int retries; + + for (retries = 0; retries < QUERY_REQ_RETRIES; retries++) { + ret = ufshcd_query_flag(hba, opcode, idn, flag_res); + if (ret) + dev_dbg(hba->dev, + "%s: failed with error %d, retries %d\n", + __func__, ret, retries); + else + break; + } + + if (ret) + dev_err(hba->dev, + "%s: query attribute, opcode %d, idn %d, failed with error %d after %d retires\n", + __func__, opcode, idn, ret, retries); + return ret; +} + /** * ufshcd_query_flag() - API function for sending flag query requests * hba: per-adapter instance @@ -1669,7 +1692,7 @@ static inline void ufshcd_init_query(struct ufs_hba *hba, * * Returns 0 for success, non-zero in case of failure */ -static int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode, +int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode, enum flag_idn idn, bool *flag_res) { struct ufs_query_req *request = NULL; @@ -2654,17 +2677,12 @@ static int ufshcd_config_pwr_mode(struct ufs_hba *hba, */ static int ufshcd_complete_dev_init(struct ufs_hba *hba) { - int i, retries, err = 0; + int i; + int err; bool flag_res = 1; - for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) { - /* Set the fDeviceInit flag */ - err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_SET_FLAG, - QUERY_FLAG_IDN_FDEVICEINIT, NULL); - if (!err || err == -ETIMEDOUT) - break; - dev_dbg(hba->dev, "%s: error %d retrying\n", __func__, err); - } + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, + QUERY_FLAG_IDN_FDEVICEINIT, NULL); if (err) { dev_err(hba->dev, "%s setting fDeviceInit flag failed with error %d\n", @@ -2672,18 +2690,11 @@ static int ufshcd_complete_dev_init(struct ufs_hba *hba) goto out; } - /* poll for max. 100 iterations for fDeviceInit flag to clear */ - for (i = 0; i < 100 && !err && flag_res; i++) { - for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) { - err = ufshcd_query_flag(hba, - UPIU_QUERY_OPCODE_READ_FLAG, - QUERY_FLAG_IDN_FDEVICEINIT, &flag_res); - if (!err || err == -ETIMEDOUT) - break; - dev_dbg(hba->dev, "%s: error %d retrying\n", __func__, - err); - } - } + /* poll for max. 1000 iterations for fDeviceInit flag to clear */ + for (i = 0; i < 1000 && !err && flag_res; i++) + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_READ_FLAG, + QUERY_FLAG_IDN_FDEVICEINIT, &flag_res); + if (err) dev_err(hba->dev, "%s reading fDeviceInit flag failed with error %d\n", @@ -3430,7 +3441,7 @@ static int ufshcd_enable_auto_bkops(struct ufs_hba *hba) if (hba->auto_bkops_enabled) goto out; - err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_SET_FLAG, + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_SET_FLAG, QUERY_FLAG_IDN_BKOPS_EN, NULL); if (err) { dev_err(hba->dev, "%s: failed to enable bkops %d\n", @@ -3479,7 +3490,7 @@ static int ufshcd_disable_auto_bkops(struct ufs_hba *hba) goto out; } - err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, + err = ufshcd_query_flag_retry(hba, UPIU_QUERY_OPCODE_CLEAR_FLAG, QUERY_FLAG_IDN_BKOPS_EN, NULL); if (err) { dev_err(hba->dev, "%s: failed to disable bkops %d\n", @@ -4450,8 +
[PATCH v7 11/14] scsi: ufs: reduce the interrupts for power mode change requests
DME commands such as Hibern8 enter/exit and gear switch generate 2 completion interrupts, one for confirmation that command is received by local UniPro and 2nd one is the final confirmation after communication with remote UniPro. Currently both of these completions are registered as interrupt events which is not quite necessary and instead we can just wait for the interrupt of 2nd completion, this should reduce the number of interrupts and could reduce the unnecessary CPU wakeups to handle extra interrupts. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 41 +++-- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 87da429..a1e9d82 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -985,13 +985,15 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) * __ufshcd_send_uic_cmd - Send UIC commands and retrieve the result * @hba: per adapter instance * @uic_cmd: UIC command + * @completion: initialize the completion only if this is set to true * * Identical to ufshcd_send_uic_cmd() expect mutex. Must be called * with mutex held and host_lock locked. * Returns 0 only if success. */ static int -__ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) +__ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd, + bool completion) { if (!ufshcd_ready_for_uic_cmd(hba)) { dev_err(hba->dev, @@ -999,7 +1001,8 @@ __ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) return -EIO; } - init_completion(&uic_cmd->done); + if (completion) + init_completion(&uic_cmd->done); ufshcd_dispatch_uic_cmd(hba, uic_cmd); @@ -1024,7 +1027,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) ufshcd_add_delay_before_dme_cmd(hba); spin_lock_irqsave(hba->host->host_lock, flags); - ret = __ufshcd_send_uic_cmd(hba, uic_cmd); + ret = __ufshcd_send_uic_cmd(hba, uic_cmd, true); spin_unlock_irqrestore(hba->host->host_lock, flags); if (!ret) ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd); @@ -2344,6 +2347,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) unsigned long flags; u8 status; int ret; + bool reenable_intr = false; mutex_lock(&hba->uic_cmd_mutex); init_completion(&uic_async_done); @@ -2351,15 +2355,17 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) spin_lock_irqsave(hba->host->host_lock, flags); hba->uic_async_done = &uic_async_done; - ret = __ufshcd_send_uic_cmd(hba, cmd); - spin_unlock_irqrestore(hba->host->host_lock, flags); - if (ret) { - dev_err(hba->dev, - "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n", - cmd->command, cmd->argument3, ret); - goto out; + if (ufshcd_readl(hba, REG_INTERRUPT_ENABLE) & UIC_COMMAND_COMPL) { + ufshcd_disable_intr(hba, UIC_COMMAND_COMPL); + /* +* Make sure UIC command completion interrupt is disabled before +* issuing UIC command. +*/ + wmb(); + reenable_intr = true; } - ret = ufshcd_wait_for_uic_cmd(hba, cmd); + ret = __ufshcd_send_uic_cmd(hba, cmd, false); + spin_unlock_irqrestore(hba->host->host_lock, flags); if (ret) { dev_err(hba->dev, "pwr ctrl cmd 0x%x with mode 0x%x uic error %d\n", @@ -2385,7 +2391,10 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba, struct uic_command *cmd) } out: spin_lock_irqsave(hba->host->host_lock, flags); + hba->active_uic_cmd = NULL; hba->uic_async_done = NULL; + if (reenable_intr) + ufshcd_enable_intr(hba, UIC_COMMAND_COMPL); spin_unlock_irqrestore(hba->host->host_lock, flags); mutex_unlock(&hba->uic_cmd_mutex); @@ -3810,16 +3819,20 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status) */ static irqreturn_t ufshcd_intr(int irq, void *__hba) { - u32 intr_status; + u32 intr_status, enabled_intr_status; irqreturn_t retval = IRQ_NONE; struct ufs_hba *hba = __hba; spin_lock(hba->host->host_lock); intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS); + enabled_intr_status = + intr_status & ufshcd_readl(hba, REG_INTERRUPT_ENABLE); - if (intr_status) { + if (intr_status) ufshcd_writel(hba, intr_status, REG_INTERRUPT_STATUS); - ufshcd_sl_intr(hba, intr_status); + + if (en
[PATCH v7 12/14] scsi: ufs: add missing memory barriers
Performing several writes to UFS host controller registers has no guarantee of ordering, so we must make sure register writes to setup request list base address etc. are performed before the run/stop register is enabled. In addition, when setting up a task request, we must make sure the updating of descriptors takes places before ringing the doorbell, similarly to setting up a transfer request. Reviewed-by: Dolev Raviv Signed-off-by: Gilad Broner Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index a1e9d82..1893a14 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -400,11 +400,9 @@ static inline int ufshcd_get_lists_status(u32 reg) * 1 UTRLRDY * 2 UTMRLRDY * 3 UCRDY -* 4 HEI -* 5 DEI -* 6-7 reserved +* 4-7 reserved */ - return (((reg) & (0xFF)) >> 1) ^ (0x07); + return ((reg & 0xFF) >> 1) ^ 0x07; } /** @@ -2724,7 +2722,7 @@ out: * To bring UFS host controller to operational state, * 1. Enable required interrupts * 2. Configure interrupt aggregation - * 3. Program UTRL and UTMRL base addres + * 3. Program UTRL and UTMRL base address * 4. Configure run-stop-registers * * Returns 0 on success, non-zero value on failure @@ -2754,8 +2752,13 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba) REG_UTP_TASK_REQ_LIST_BASE_H); /* +* Make sure base address and interrupt setup are updated before +* enabling the run/stop registers below. +*/ + wmb(); + + /* * UCRDY, UTMRLDY and UTRLRDY bits must be 1 -* DEI, HEI bits must be 0 */ reg = ufshcd_readl(hba, REG_CONTROLLER_STATUS); if (!(ufshcd_get_lists_status(reg))) { @@ -3918,6 +3921,10 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id, /* send command to the controller */ __set_bit(free_slot, &hba->outstanding_tasks); + + /* Make sure descriptors are ready before ringing the task doorbell */ + wmb(); + ufshcd_writel(hba, 1 << free_slot, REG_UTP_TASK_REQ_DOOR_BELL); spin_unlock_irqrestore(host->host_lock, flags); -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 14/14] scsi: ufs: add wrapper for retrying sending query attribute
Sometimes queries from the device might return a failure so it is recommended to retry sending the query, before giving up. This change adds a wrapper to retry sending a query attribute, in cases where we need to wait longer, before we continue, or before reporting a failure. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 51 --- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 2fc678d..7ab6a45 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1821,6 +1821,43 @@ out: } /** + * ufshcd_query_attr_retry() - API function for sending query + * attribute with retries + * @hba: per-adapter instance + * @opcode: attribute opcode + * @idn: attribute idn to access + * @index: index field + * @selector: selector field + * @attr_val: the attribute value after the query request + * completes + * + * Returns 0 for success, non-zero in case of failure +*/ +static int ufshcd_query_attr_retry(struct ufs_hba *hba, + enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector, + u32 *attr_val) +{ + int ret = 0; + u32 retries; + +for (retries = QUERY_REQ_RETRIES; retries > 0; retries--) { + ret = ufshcd_query_attr(hba, opcode, idn, index, + selector, attr_val); + if (ret) + dev_dbg(hba->dev, "%s: failed with error %d, retries %d\n", + __func__, ret, retries); + else + break; + } + + if (ret) + dev_err(hba->dev, + "%s: query attribute, idn %d, failed with error %d after %d retires\n", + __func__, idn, ret, QUERY_REQ_RETRIES); + return ret; +} + +/** * ufshcd_query_descriptor - API function for sending descriptor requests * hba: per-adapter instance * opcode: attribute opcode @@ -3401,7 +3438,7 @@ static int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask) val = hba->ee_ctrl_mask & ~mask; val &= 0x; /* 2 bytes */ - err = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, + err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val); if (!err) hba->ee_ctrl_mask &= ~mask; @@ -3429,7 +3466,7 @@ static int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask) val = hba->ee_ctrl_mask | mask; val &= 0x; /* 2 bytes */ - err = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, + err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &val); if (!err) hba->ee_ctrl_mask |= mask; @@ -3535,7 +3572,7 @@ static void ufshcd_force_reset_auto_bkops(struct ufs_hba *hba) static inline int ufshcd_get_bkops_status(struct ufs_hba *hba, u32 *status) { - return ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR, + return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, QUERY_ATTR_IDN_BKOPS_STATUS, 0, 0, status); } @@ -3598,7 +3635,7 @@ static int ufshcd_urgent_bkops(struct ufs_hba *hba) static inline int ufshcd_get_ee_status(struct ufs_hba *hba, u32 *status) { - return ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR, + return ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR, QUERY_ATTR_IDN_EE_STATUS, 0, 0, status); } @@ -4352,9 +4389,9 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba) dev_dbg(hba->dev, "%s: setting icc_level 0x%x", __func__, hba->init_prefetch_data.icc_level); - ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, - QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, - &hba->init_prefetch_data.icc_level); + ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, + QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, + &hba->init_prefetch_data.icc_level); if (ret) dev_err(hba->dev, -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/14] scsi: ufs: clear UTRD, UPIU req and rsp before new transfers
Clear the UFS data structures before sending new request. The SCSI command is sent to the device within the UFS UPIU request. As part of the transfer UPIU preparation, the SCSI command is copied to the UPIU structure according to the SCSI command size. As different SCSI commands differ in size from each other, we need to clear the whole SCSI command field to prevent sending uninitialized data to the device. The UPIU response doesn't always include the sense data and can differ in size. Hence, the UPIU response should also be cleared before the transfer. Reviewed-by: Gilad Broner Reviewed-by: Dolev Raviv Signed-off-by: Subhash Jadavani Signed-off-by: Maya Erez Signed-off-by: Yaniv Gardi --- drivers/scsi/ufs/ufshcd.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 85cd256..03533f0 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3,7 +3,7 @@ * * This code is based on drivers/scsi/ufs/ufshcd.c * Copyright (C) 2011-2013 Samsung India Software Operations - * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved. + * Copyright (c) 2013-2016, The Linux Foundation. All rights reserved. * * Authors: * Santosh Yaraganavi @@ -1035,6 +1035,7 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) cpu_to_le32(lower_32_bits(sg->dma_address)); prd_table[i].upper_addr = cpu_to_le32(upper_32_bits(sg->dma_address)); + prd_table[i].reserved = 0; } } else { lrbp->utr_descriptor_ptr->prd_table_length = 0; @@ -1117,7 +1118,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp, /* Transfer request descriptor header fields */ req_desc->header.dword_0 = cpu_to_le32(dword_0); - + /* dword_1 is reserved, hence it is set to 0 */ + req_desc->header.dword_1 = 0; /* * assigning invalid value for command status. Controller * updates OCS on command completion, with the command @@ -1125,6 +1127,8 @@ static void ufshcd_prepare_req_desc_hdr(struct ufshcd_lrb *lrbp, */ req_desc->header.dword_2 = cpu_to_le32(OCS_INVALID_COMMAND_STATUS); + /* dword_3 is reserved, hence it is set to 0 */ + req_desc->header.dword_3 = 0; } /** @@ -1137,6 +1141,7 @@ static void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u32 upiu_flags) { struct utp_upiu_req *ucd_req_ptr = lrbp->ucd_req_ptr; + unsigned short cdb_len; /* command descriptor fields */ ucd_req_ptr->header.dword_0 = UPIU_HEADER_DWORD( @@ -1151,8 +1156,11 @@ void ufshcd_prepare_utp_scsi_cmd_upiu(struct ufshcd_lrb *lrbp, u32 upiu_flags) ucd_req_ptr->sc.exp_data_transfer_len = cpu_to_be32(lrbp->cmd->sdb.length); - memcpy(ucd_req_ptr->sc.cdb, lrbp->cmd->cmnd, - (min_t(unsigned short, lrbp->cmd->cmd_len, MAX_CDB_SIZE))); + cdb_len = min_t(unsigned short, lrbp->cmd->cmd_len, MAX_CDB_SIZE); + memset(ucd_req_ptr->sc.cdb, 0, MAX_CDB_SIZE); + memcpy(ucd_req_ptr->sc.cdb, lrbp->cmd->cmnd, cdb_len); + + memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp)); } /** -- 1.8.5.2 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add support for DWC UFS Host Controller
Please consider this patch. This patch includes: - quirks in the ufs core driver to support Synopsys MPHY Test Chip config - quirks in the ufs core driver to support DWC configuration sequence - New Unipro attributes were added - ufs core driver was tweaked to support UFS 2.0 - support for Synopsys PCI ID in the pci glue driver - new platform glue driver for Synopsys devices Signed-off-by: Joao Pinto --- Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 16 + drivers/scsi/ufs/Kconfig | 54 ++ drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-dwc.c| 95 +++ drivers/scsi/ufs/ufshcd-pci.c | 2 + drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +- drivers/scsi/ufs/ufshcd.c | 822 +- drivers/scsi/ufs/ufshcd.h | 15 + drivers/scsi/ufs/ufshci.h | 29 + drivers/scsi/ufs/unipro.h | 39 + 10 files changed, 1069 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt create mode 100644 drivers/scsi/ufs/ufs-dwc.c diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file mode 100644 index 000..fa361f2 --- /dev/null +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt @@ -0,0 +1,16 @@ +* Universal Flash Storage (UFS) DesignWare Host Controller + +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers. +Each UFS controller instance should have its own node. + +Required properties: +- compatible: compatible list, contains "snps,ufshcd" +- reg : +- interrupts: + +Example: + dwc_ufshcd@0xD000 { + compatible = "snps,ufshcd"; + reg = < 0xD000 0x1 >; + interrupts = < 24 >; + }; diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig index 5f45307..5da4b8f 100644 --- a/drivers/scsi/ufs/Kconfig +++ b/drivers/scsi/ufs/Kconfig @@ -83,3 +83,57 @@ config SCSI_UFS_QCOM Select this if you have UFS controller on QCOM chipset. If unsure, say N. + +config SCSI_UFS_DWC_HOOKS + bool "DesignWare hooks to UFS controller" + depends on SCSI_UFSHCD + ---help--- + This selects the DesignWare hooks for the UFS host controller. + + Select this if you have a DesignWare UFS controller. + If unsure, say N. + +config SCSI_UFS_DWC_PLAT + tristate "DesignWare UFS controller platform glue driver" + depends on SCSI_UFS_DWC_HOOKS && SCSI_UFSHCD_PLATFORM + ---help--- + This selects the DesignWare UFS host controller platform glue driver. + + Select this if you have a DesignWare UFS controller on Platform bus. + If unsure, say N. + +config SCSI_UFS_DWC_MPHY_TC + bool "Support for the Synopsys MPHY Test Chip" + depends on SCSI_UFS_DWC_PLAT + ---help--- + This selects the support for the Synopsys MPHY Test Chip. + + Select this if you have a Synopsys MPHY Test Chip. + If unsure, say N. + +config SCSI_UFS_DWC_MPHY_TC_GEN2 + bool "Support for the Synopsys MPHY Test Chip Gen2" + depends on SCSI_UFS_DWC_20BIT_RMMI || SCSI_UFS_DWC_40BIT_RMMI + ---help--- + This selects the support for the Synopsys MPHY Test Chip Gen2. + + Select this if you have a Synopsys MPHY Test Chip Gen2. + If unsure, say N. + +config SCSI_UFS_DWC_20BIT_RMMI + bool "20-bit RMMI MPHY" + depends on SCSI_UFS_DWC_MPHY_TC + ---help--- + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. + + Select this if you are using a 40-bit RMMI Synopsys MPHY. + If unsure, say N. + +config SCSI_UFS_DWC_40BIT_RMMI + bool "40-bit RMMI MPHY" + depends on SCSI_UFS_DWC_MPHY_TC + ---help--- + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. + + Select this if you are using a 40-bit RMMI Synopsys MPHY. + If unsure, say N. diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile index 8303bcc..c14b9e3 100644 --- a/drivers/scsi/ufs/Makefile +++ b/drivers/scsi/ufs/Makefile @@ -1,4 +1,5 @@ # UFSHCD makefile +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o diff --git a/drivers/scsi/ufs/ufs-dwc.c b/drivers/scsi/ufs/ufs-dwc.c new file mode 100644 index 000..e1e469f --- /dev/null +++ b/drivers/scsi/ufs/ufs-dwc.c @@ -0,0 +1,95 @@ +/* + * UFS Host driver for Synopsys Designware Core + * + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com) + * + * Authors: Joao Pinto + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License ver
Re: megaraid_sas: add an i/o barrier
On 1.2.2016 05:45, Kashyap Desai wrote: >> -Original Message- >> From: Tomas Henzl [mailto:the...@redhat.com] >> Sent: Friday, January 29, 2016 11:05 PM >> To: 'linux-scsi@vger.kernel.org' >> Cc: sumit.sax...@avagotech.com; Desai, Kashyap; Uday Lingala >> Subject: megaraid_sas: add an i/o barrier >> >> A barrier should be added to ensure proper ordering of memory mapped >> writes. >> >> Signed-off-by: Tomas Henzl >> --- >> drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c >> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >> index d9d0029fb1..98a848bdfd 100644 >> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >> @@ -204,6 +204,7 @@ megasas_fire_cmd_fusion(struct >> megasas_instance *instance, >> &instance->reg_set->inbound_low_queue_port); >> writel(le32_to_cpu(req_desc->u.high), >> &instance->reg_set->inbound_high_queue_port); >> +mmiowb(); >> spin_unlock_irqrestore(&instance->hba_lock, flags); #endif } > Tomas- > > We may need similar changes around below Functions as well, because there is > no associated readX or mmiowb() call. > megasas_fire_cmd_xscale() > megasas_fire_cmd_ppc() > megasas_fire_cmd_skinny() > megasas_fire_cmd_gen2() > > Also, wrireq() routine in same function megasas_fire_cmd_fusion() need i/o > barrier. I don't think so (with the exception of megasas_fire_cmd_skinny - I missed this one). When two threads try to use a fire_cmd there is no protection of certain ordering, that had to be done in a caller of fire_cmd (for example in megasas_build_and_issue_cmd_fusion) and it seems to me that there is nothing like that. Likely is, that this - a strict ordering of commands - is not needed. The protection which I'm adding is needed when a command consist of a sequence of more than one write, see memory-barriers.txt. (It looks to me that megasas_fire_cmd_ -xscale -ppc -skiny -gen2 do not need the hba_lock unless there is another i/o sequence protected with the same lock (note also that there is no such lock in megasas_fire_cmd_fusion).) I'll add the mmiowb to megasas_fire_cmd_skinny and send a new patch - agreed? --tms > >> -- >> 2.4.3 > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add support for DWC UFS Host Controller
This patch includes: - quirks in the ufs core driver to support Synopsys MPHY Test Chip config - quirks in the ufs core driver to support DWC configuration sequence - New Unipro attributes were added - ufs core driver was tweaked to support UFS 2.0 - support for Synopsys PCI ID in the pci glue driver - new platform glue driver for Synopsys devices Signed-off-by: Joao Pinto --- Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 16 + drivers/scsi/ufs/Kconfig | 54 ++ drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-dwc.c| 115 +++ drivers/scsi/ufs/ufshcd-pci.c | 2 + drivers/scsi/ufs/ufshcd-pltfrm.c | 2 +- drivers/scsi/ufs/ufshcd.c | 822 +- drivers/scsi/ufs/ufshcd.h | 15 + drivers/scsi/ufs/ufshci.h | 29 + drivers/scsi/ufs/unipro.h | 39 + 10 files changed, 1089 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/ufs/ufs-dwc.txt create mode 100644 drivers/scsi/ufs/ufs-dwc.c diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt new file mode 100644 index 000..fa361f2 --- /dev/null +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt @@ -0,0 +1,16 @@ +* Universal Flash Storage (UFS) DesignWare Host Controller + +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers. +Each UFS controller instance should have its own node. + +Required properties: +- compatible: compatible list, contains "snps,ufshcd" +- reg : +- interrupts: + +Example: + dwc_ufshcd@0xD000 { + compatible = "snps,ufshcd"; + reg = < 0xD000 0x1 >; + interrupts = < 24 >; + }; diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig index 5f45307..5da4b8f 100644 --- a/drivers/scsi/ufs/Kconfig +++ b/drivers/scsi/ufs/Kconfig @@ -83,3 +83,57 @@ config SCSI_UFS_QCOM Select this if you have UFS controller on QCOM chipset. If unsure, say N. + +config SCSI_UFS_DWC_HOOKS + bool "DesignWare hooks to UFS controller" + depends on SCSI_UFSHCD + ---help--- + This selects the DesignWare hooks for the UFS host controller. + + Select this if you have a DesignWare UFS controller. + If unsure, say N. + +config SCSI_UFS_DWC_PLAT + tristate "DesignWare UFS controller platform glue driver" + depends on SCSI_UFS_DWC_HOOKS && SCSI_UFSHCD_PLATFORM + ---help--- + This selects the DesignWare UFS host controller platform glue driver. + + Select this if you have a DesignWare UFS controller on Platform bus. + If unsure, say N. + +config SCSI_UFS_DWC_MPHY_TC + bool "Support for the Synopsys MPHY Test Chip" + depends on SCSI_UFS_DWC_PLAT + ---help--- + This selects the support for the Synopsys MPHY Test Chip. + + Select this if you have a Synopsys MPHY Test Chip. + If unsure, say N. + +config SCSI_UFS_DWC_MPHY_TC_GEN2 + bool "Support for the Synopsys MPHY Test Chip Gen2" + depends on SCSI_UFS_DWC_20BIT_RMMI || SCSI_UFS_DWC_40BIT_RMMI + ---help--- + This selects the support for the Synopsys MPHY Test Chip Gen2. + + Select this if you have a Synopsys MPHY Test Chip Gen2. + If unsure, say N. + +config SCSI_UFS_DWC_20BIT_RMMI + bool "20-bit RMMI MPHY" + depends on SCSI_UFS_DWC_MPHY_TC + ---help--- + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. + + Select this if you are using a 40-bit RMMI Synopsys MPHY. + If unsure, say N. + +config SCSI_UFS_DWC_40BIT_RMMI + bool "40-bit RMMI MPHY" + depends on SCSI_UFS_DWC_MPHY_TC + ---help--- + This specifies that the Synopsys MPHY supports 40-bit RMMI operations. + + Select this if you are using a 40-bit RMMI Synopsys MPHY. + If unsure, say N. diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile index 8303bcc..c14b9e3 100644 --- a/drivers/scsi/ufs/Makefile +++ b/drivers/scsi/ufs/Makefile @@ -1,4 +1,5 @@ # UFSHCD makefile +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o diff --git a/drivers/scsi/ufs/ufs-dwc.c b/drivers/scsi/ufs/ufs-dwc.c new file mode 100644 index 000..e4d70b7 --- /dev/null +++ b/drivers/scsi/ufs/ufs-dwc.c @@ -0,0 +1,115 @@ +/* == + * The Synopsys DWC UFS Software Driver and documentation (hereinafter + * "Software") is an unsupported proprietary work of Synopsys, Inc. unless + * otherwise expressly agreed to in writing between Synopsys and you. + * + * The Sof
Re: [PATCH 11/12] be2iscsi: _bh for io_sgl_lock and mgmt_sgl_lock
On Mon, Feb 01, 2016 at 03:42:50PM +0530, Jitendra Bhivare wrote: > Processing of mgmt and IO tasks are done in process context and sofitrqs. > > Allocation and freeing of sgl_handles needs to be done under > spin_lock_bh/spin_unlock_bh and move the locks to the routines. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_main.c | 25 ++--- > 1 file changed, 10 insertions(+), 15 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c > index 03265b6..fa2b589 100644 > --- a/drivers/scsi/be2iscsi/be_main.c > +++ b/drivers/scsi/be2iscsi/be_main.c > @@ -1132,6 +1132,7 @@ static struct sgl_handle *alloc_io_sgl_handle(struct > beiscsi_hba *phba) > { > struct sgl_handle *psgl_handle; > > + spin_lock_bh(&phba->io_sgl_lock); > if (phba->io_sgl_hndl_avbl) { > beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO, > "BM_%d : In alloc_io_sgl_handle," > @@ -1149,12 +1150,14 @@ static struct sgl_handle *alloc_io_sgl_handle(struct > beiscsi_hba *phba) > phba->io_sgl_alloc_index++; > } else > psgl_handle = NULL; > + spin_unlock_bh(&phba->io_sgl_lock); > return psgl_handle; > } > > static void > free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) > { > + spin_lock_bh(&phba->io_sgl_lock); > beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO, > "BM_%d : In free_,io_sgl_free_index=%d\n", > phba->io_sgl_free_index); > @@ -1169,6 +1172,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct > sgl_handle *psgl_handle) >"value there=%p\n", phba->io_sgl_free_index, >phba->io_sgl_hndl_base >[phba->io_sgl_free_index]); > + spin_unlock_bh(&phba->io_sgl_lock); > return; > } > phba->io_sgl_hndl_base[phba->io_sgl_free_index] = psgl_handle; > @@ -1177,6 +1181,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct > sgl_handle *psgl_handle) > phba->io_sgl_free_index = 0; > else > phba->io_sgl_free_index++; > + spin_unlock_bh(&phba->io_sgl_lock); > } > > static inline struct wrb_handle * > @@ -1257,6 +1262,7 @@ static struct sgl_handle *alloc_mgmt_sgl_handle(struct > beiscsi_hba *phba) > { > struct sgl_handle *psgl_handle; > > + spin_lock_bh(&phba->mgmt_sgl_lock); > if (phba->eh_sgl_hndl_avbl) { > psgl_handle = phba->eh_sgl_hndl_base[phba->eh_sgl_alloc_index]; > phba->eh_sgl_hndl_base[phba->eh_sgl_alloc_index] = NULL; > @@ -1274,13 +1280,14 @@ static struct sgl_handle > *alloc_mgmt_sgl_handle(struct beiscsi_hba *phba) > phba->eh_sgl_alloc_index++; > } else > psgl_handle = NULL; > + spin_unlock_bh(&phba->mgmt_sgl_lock); > return psgl_handle; > } > > void > free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle > *psgl_handle) > { > - > + spin_lock_bh(&phba->mgmt_sgl_lock); > beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, > "BM_%d : In free_mgmt_sgl_handle," > "eh_sgl_free_index=%d\n", > @@ -1295,6 +1302,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct > sgl_handle *psgl_handle) > "BM_%d : Double Free in eh SGL ," > "eh_sgl_free_index=%d\n", > phba->eh_sgl_free_index); > + spin_unlock_bh(&phba->mgmt_sgl_lock); > return; > } > phba->eh_sgl_hndl_base[phba->eh_sgl_free_index] = psgl_handle; > @@ -1304,6 +1312,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct > sgl_handle *psgl_handle) > phba->eh_sgl_free_index = 0; > else > phba->eh_sgl_free_index++; > + spin_unlock_bh(&phba->mgmt_sgl_lock); > } > > static void > @@ -4616,11 +4625,9 @@ beiscsi_free_mgmt_task_handles(struct beiscsi_conn > *beiscsi_conn, > } > > if (io_task->psgl_handle) { > - spin_lock_bh(&phba->mgmt_sgl_lock); > free_mgmt_sgl_handle(phba, >io_task->psgl_handle); > io_task->psgl_handle = NULL; > - spin_unlock_bh(&phba->mgmt_sgl_lock); > } > > if (io_task->mtask_addr) { > @@ -4666,9 +4673,7 @@ static void beiscsi_cleanup_task(struct iscsi_task > *task) > } > > if (io_task->psgl_handle) { > - spin_lock(&phba->io_sgl_lock); > free_io_sgl_handle(phba, io_task->psgl_handle); > - spin_unlock(&phba->io_sgl_lock); > io_task->psgl_handle = NULL; > } > > @@ -4784,9 +4789,7 @@ static int beiscsi_alloc_pdu(struct iscsi_task *task, > uint8_t opcode) > io_task->pwrb_handle = NULL; >
Re: [PATCH 12/12] be2iscsi: Add lock to protect WRB alloc and free
On Mon, Feb 01, 2016 at 03:42:51PM +0530, Jitendra Bhivare wrote: > FW got into UE after running IO stress test > > With kernel change to split session lock in frwd_lock and back_lock for tx > and rx path correspondingly, in the IO path, common resource used in driver > such as WRB was left unprotected. > > Add wrb_lock spinlock to protect allocation and freeing of WRB. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_main.c | 5 + > drivers/scsi/be2iscsi/be_main.h | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c > index fa2b589..0892ee2 100644 > --- a/drivers/scsi/be2iscsi/be_main.c > +++ b/drivers/scsi/be2iscsi/be_main.c > @@ -1190,12 +1190,14 @@ beiscsi_get_wrb_handle(struct hwi_wrb_context > *pwrb_context, > { > struct wrb_handle *pwrb_handle; > > + spin_lock_bh(&pwrb_context->wrb_lock); > pwrb_handle = pwrb_context->pwrb_handle_base[pwrb_context->alloc_index]; > pwrb_context->wrb_handles_available--; > if (pwrb_context->alloc_index == (wrbs_per_cxn - 1)) > pwrb_context->alloc_index = 0; > else > pwrb_context->alloc_index++; > + spin_unlock_bh(&pwrb_context->wrb_lock); > > return pwrb_handle; > } > @@ -1227,12 +1229,14 @@ beiscsi_put_wrb_handle(struct hwi_wrb_context > *pwrb_context, > struct wrb_handle *pwrb_handle, > unsigned int wrbs_per_cxn) > { > + spin_lock_bh(&pwrb_context->wrb_lock); > pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle; > pwrb_context->wrb_handles_available++; > if (pwrb_context->free_index == (wrbs_per_cxn - 1)) > pwrb_context->free_index = 0; > else > pwrb_context->free_index++; > + spin_unlock_bh(&pwrb_context->wrb_lock); > } > > /** > @@ -2920,6 +2924,7 @@ static int beiscsi_init_wrb_handle(struct beiscsi_hba > *phba) > } > num_cxn_wrbh--; > } > + spin_lock_init(&pwrb_context->wrb_lock); > } > idx = 0; > for (index = 0; index < phba->params.cxns_per_ctrl; index++) { > diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h > index 5ded3fa..30a4606 100644 > --- a/drivers/scsi/be2iscsi/be_main.h > +++ b/drivers/scsi/be2iscsi/be_main.h > @@ -304,6 +304,7 @@ struct invalidate_command_table { > #define BEISCSI_GET_ULP_FROM_CRI(phwi_ctrlr, cri) \ > (phwi_ctrlr->wrb_context[cri].ulp_num) > struct hwi_wrb_context { > + spinlock_t wrb_lock; > struct list_head wrb_handle_list; > struct list_head wrb_handle_drvr_list; > struct wrb_handle **pwrb_handle_base; > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/12] be2iscsi: Rename MCC and BMBX processing functions
Hi Jitendra, [auto build test ERROR on scsi/for-next] [also build test ERROR on next-20160201] [cannot apply to v4.5-rc2] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Jitendra-Bhivare/be2iscsi-critical-fixes-for-11-0-0-0/20160201-181716 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: x86_64-randconfig-s1-02011933 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 Note: the linux-review/Jitendra-Bhivare/be2iscsi-critical-fixes-for-11-0-0-0/20160201-181716 HEAD 7dd8e579ae5fffda523c8c75978134fe99d5aa68 builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): drivers/scsi/be2iscsi/be_cmds.c: In function 'be_mbox_notify_wait': >> drivers/scsi/be2iscsi/be_cmds.c:716:12: error: implicit declaration of >> function 'be_mcc_compl_process' [-Werror=implicit-function-declaration] status = be_mcc_compl_process(ctrl, &mbox->compl); ^ cc1: some warnings being treated as errors vim +/be_mcc_compl_process +716 drivers/scsi/be2iscsi/be_cmds.c ea53ed97 Jitendra Bhivare2016-02-01 710status = be_mbox_db_ready_poll(ctrl); bfead3b2 Jayamohan Kallickal 2009-10-23 711if (status != 0) bfead3b2 Jayamohan Kallickal 2009-10-23 712return status; bfead3b2 Jayamohan Kallickal 2009-10-23 713 bfead3b2 Jayamohan Kallickal 2009-10-23 714/* A cq entry has been made now */ bfead3b2 Jayamohan Kallickal 2009-10-23 715if (be_mcc_compl_is_new(compl)) { bfead3b2 Jayamohan Kallickal 2009-10-23 @716status = be_mcc_compl_process(ctrl, &mbox->compl); bfead3b2 Jayamohan Kallickal 2009-10-23 717be_mcc_compl_use(compl); bfead3b2 Jayamohan Kallickal 2009-10-23 718if (status) bfead3b2 Jayamohan Kallickal 2009-10-23 719return status; :: The code at line 716 was first introduced by commit :: bfead3b2cb4607c71831423c3ee97d22cd0c9dcb [SCSI] be2iscsi: Adding msix and mcc_rings V3 :: TO: Jayamohan Kallickal :: CC: James Bottomley --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 10/12] be2iscsi: Fix ExpStatSn in management tasks
On Mon, Feb 01, 2016 at 03:42:49PM +0530, Jitendra Bhivare wrote: > Connection resets observed from some targets when NOP-Out with wrong > ExpStatSn is sent. > > FW keeps track of StatSn and fills up ExpStatSn accordingly. > The header filled up by the stack needs to be modified by driver to clear > ExpStatSn. If the field is not cleared, FW recalculates ExpStatSn and > wrong offset'ed ExpStatSn is seen in the wire trace. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_main.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c > index 3f08a11..03265b6 100644 > --- a/drivers/scsi/be2iscsi/be_main.c > +++ b/drivers/scsi/be2iscsi/be_main.c > @@ -4926,7 +4926,6 @@ int beiscsi_iotask_v2(struct iscsi_task *task, struct > scatterlist *sg, > > pwrb = io_task->pwrb_handle->pwrb; > > - io_task->cmd_bhs->iscsi_hdr.exp_statsn = 0; > io_task->bhs_len = sizeof(struct be_cmd_bhs); > > if (writedir) { > @@ -4987,7 +4986,6 @@ static int beiscsi_iotask(struct iscsi_task *task, > struct scatterlist *sg, > unsigned int doorbell = 0; > > pwrb = io_task->pwrb_handle->pwrb; > - io_task->cmd_bhs->iscsi_hdr.exp_statsn = 0; > io_task->bhs_len = sizeof(struct be_cmd_bhs); > > if (writedir) { > @@ -5159,23 +5157,21 @@ static int beiscsi_task_xmit(struct iscsi_task *task) > { > struct beiscsi_io_task *io_task = task->dd_data; > struct scsi_cmnd *sc = task->sc; > - struct beiscsi_hba *phba = NULL; > + struct beiscsi_hba *phba; > struct scatterlist *sg; > int num_sg; > unsigned int writedir = 0, xferlen = 0; > > - phba = ((struct beiscsi_conn *)task->conn->dd_data)->phba; > + if (!io_task->conn->login_in_progress) > + task->hdr->exp_statsn = 0; > > if (!sc) > return beiscsi_mtask(task); > > io_task->scsi_cmnd = sc; > num_sg = scsi_dma_map(sc); > + phba = io_task->conn->phba; > if (num_sg < 0) { > - struct iscsi_conn *conn = task->conn; > - struct beiscsi_hba *phba = NULL; > - > - phba = ((struct beiscsi_conn *)conn->dd_data)->phba; > beiscsi_log(phba, KERN_ERR, > BEISCSI_LOG_IO | BEISCSI_LOG_ISCSI, > "BM_%d : scsi_dma_map Failed " > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] be2iscsi: Couple MCC tag and WRB alloc and free
On Mon, Feb 01, 2016 at 03:42:48PM +0530, Jitendra Bhivare wrote: > WARN_ON(atomic_read(&mccq->used) >= mccq->len) seen when FW gets into UE. > > MCCQ overflow is happening because driver discards any new request and > frees up the tag. The tag allocation controls the number of MCC WRB posted. > It is being replenished but WRBs are not hence the WARN_ON. > > Allocation and freeing of WRB and tags for MCC is now done in one place. > This helps to achieve proper accounting of WRB indices and MCC tags. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be.h | 2 +- > drivers/scsi/be2iscsi/be_cmds.c | 103 +- > drivers/scsi/be2iscsi/be_cmds.h | 6 +- > drivers/scsi/be2iscsi/be_main.c | 3 +- > drivers/scsi/be2iscsi/be_mgmt.c | 134 > +++- > 5 files changed, 130 insertions(+), 118 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h > index da1d87a..ee5ace8 100644 > --- a/drivers/scsi/be2iscsi/be.h > +++ b/drivers/scsi/be2iscsi/be.h > @@ -42,7 +42,7 @@ struct be_queue_info { > u16 id; > u16 tail, head; > bool created; > - atomic_t used; /* Number of valid elements in the queue */ > + u16 used; /* Number of valid elements in the queue */ > }; > > static inline u32 MODULO(u16 val, u16 limit) > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index 728aa133..a55eaee 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -126,8 +126,62 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > return tag; > } > > -void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) > +struct be_mcc_wrb *alloc_mcc_wrb(struct beiscsi_hba *phba, > + unsigned int *ref_tag) > { > + struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; > + struct be_mcc_wrb *wrb = NULL; > + unsigned int tag; > + > + spin_lock_bh(&phba->ctrl.mcc_lock); > + if (mccq->used == mccq->len) { > + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | > + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > + "BC_%d : MCC queue full: WRB used %u tag avail > %u\n", > + mccq->used, phba->ctrl.mcc_tag_available); > + goto alloc_failed; > + } > + > + if (!phba->ctrl.mcc_tag_available) > + goto alloc_failed; > + > + tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index]; > + if (!tag) { > + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | > + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > + "BC_%d : MCC tag 0 allocated: tag avail %u alloc > index %u\n", > + phba->ctrl.mcc_tag_available, > + phba->ctrl.mcc_alloc_index); > + goto alloc_failed; > + } > + > + /* return this tag for further reference */ > + *ref_tag = tag; > + phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0; > + phba->ctrl.mcc_tag_status[tag] = 0; > + phba->ctrl.ptag_state[tag].tag_state = 0; > + phba->ctrl.mcc_tag_available--; > + if (phba->ctrl.mcc_alloc_index == (MAX_MCC_CMD - 1)) > + phba->ctrl.mcc_alloc_index = 0; > + else > + phba->ctrl.mcc_alloc_index++; > + > + wrb = queue_head_node(mccq); > + memset(wrb, 0, sizeof(*wrb)); > + wrb->tag0 = tag; > + wrb->tag0 |= (mccq->head << MCC_Q_WRB_IDX_SHIFT) & MCC_Q_WRB_IDX_MASK; > + queue_head_inc(mccq); > + mccq->used++; > + > +alloc_failed: > + spin_unlock_bh(&phba->ctrl.mcc_lock); > + return wrb; > +} > + > +void free_mcc_wrb(struct be_ctrl_info *ctrl, unsigned int tag) > +{ > + struct be_queue_info *mccq = &ctrl->mcc_obj.q; > + > spin_lock_bh(&ctrl->mcc_lock); > tag = tag & MCC_Q_CMD_TAG_MASK; > ctrl->mcc_tag[ctrl->mcc_free_index] = tag; > @@ -136,6 +190,7 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int > tag) > else > ctrl->mcc_free_index++; > ctrl->mcc_tag_available++; > + mccq->used--; > spin_unlock_bh(&ctrl->mcc_lock); > } > > @@ -173,10 +228,8 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, > struct be_cmd_resp_hdr *mbx_resp_hdr; > struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; > > - if (beiscsi_error(phba)) { > - free_mcc_tag(&phba->ctrl, tag); > + if (beiscsi_error(phba)) > return -EPERM; > - } > > /* wait for the mccq completion */ > rc = wait_event_interruptible_timeout( > @@ -259,7 +312,7 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, > } > } > > - free_mcc_tag(&phba->ctrl, tag); > + free_mcc_wrb(&phba->ctrl, tag); > return rc; > } > > @@ -479,7 +532,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, >
Re: scsi_debug, iosize optimal < minimum
> "Ruediger" == Ruediger Meier writes: Ruediger> I've noticed a possible problem with scsi_debug devices where Ruediger> optimal_io_size < minimum_io_size. Ruediger> The problem seems reproducable with kernel 4.4.0 and also with Ruediger> stable version 4.1.15. For stable tree the last known working Ruediger> version is 4.1.13. http://git.kernel.org/cgit/linux/kernel/git/mkp/scsi.git/commit/?h=4.5/scsi-fixes&id=d0eb20a863ba7dc1d3f4b841639671f134560be2 -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/12] be2iscsi: Fix MCC WRB leak in open_connection
On Mon, Feb 01, 2016 at 03:42:47PM +0530, Jitendra Bhivare wrote: > In open with IP of unknown address family, only tag is freed and error > returned. MCC WRB allocated for the operation is not freed. > > Added check for supported family of IP in the beginning before > allocating the tag and WRB. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_mgmt.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c > index 85044b8..ccac1d7 100644 > --- a/drivers/scsi/be2iscsi/be_mgmt.c > +++ b/drivers/scsi/be2iscsi/be_mgmt.c > @@ -829,6 +829,13 @@ int mgmt_open_connection(struct beiscsi_hba *phba, > unsigned short cid = beiscsi_ep->ep_cid; > struct be_sge *sge; > > + if (dst_addr->sa_family != PF_INET && dst_addr->sa_family != PF_INET6) { > + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, > + "BG_%d : unknown addr family %d\n", > + dst_addr->sa_family); > + return -EINVAL; > + } > + > phwi_ctrlr = phba->phwi_ctrlr; > phwi_context = phwi_ctrlr->phwi_ctxt; > > @@ -868,7 +875,8 @@ int mgmt_open_connection(struct beiscsi_hba *phba, > beiscsi_ep->dst_addr = daddr_in->sin_addr.s_addr; > beiscsi_ep->dst_tcpport = ntohs(daddr_in->sin_port); > beiscsi_ep->ip_type = BE2_IPV4; > - } else if (dst_addr->sa_family == PF_INET6) { > + } else { > + /* else its PF_INET6 family */ > req->ip_address.ip_type = BE2_IPV6; > memcpy(&req->ip_address.addr, > &daddr_in6->sin6_addr.in6_u.u6_addr8, 16); > @@ -877,14 +885,6 @@ int mgmt_open_connection(struct beiscsi_hba *phba, > memcpy(&beiscsi_ep->dst6_addr, > &daddr_in6->sin6_addr.in6_u.u6_addr8, 16); > beiscsi_ep->ip_type = BE2_IPV6; > - } else{ > - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, > - "BG_%d : unknown addr family %d\n", > - dst_addr->sa_family); > - mutex_unlock(&ctrl->mbox_lock); > - free_mcc_tag(&phba->ctrl, tag); > - return -EINVAL; > - > } > req->cid = cid; > i = phba->nxt_cqid++; > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/12] be2iscsi: Cleanup processing of BMBX completion
On Mon, Feb 01, 2016 at 03:42:46PM +0530, Jitendra Bhivare wrote: > Remove confusingly named be_mcc_compl_is_new and be_mcc_compl_use functions > in processing of BMBX. Rearrange beiscsi_process_mbox_compl function. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_cmds.c | 75 > - > 1 file changed, 36 insertions(+), 39 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index 60db2de..728aa133 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -263,21 +263,6 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, > return rc; > } > > -static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl) > -{ > - if (compl->flags != 0) { > - compl->flags = le32_to_cpu(compl->flags); > - WARN_ON((compl->flags & CQE_FLAGS_VALID_MASK) == 0); > - return true; > - } else > - return false; > -} > - > -static inline void be_mcc_compl_use(struct be_mcc_compl *compl) > -{ > - compl->flags = 0; > -} > - > /* > * beiscsi_process_mbox_compl()- Check the MBX completion status > * @ctrl: Function specific MBX data structure > @@ -298,30 +283,46 @@ static int beiscsi_process_mbox_compl(struct > be_ctrl_info *ctrl, > struct be_cmd_req_hdr *hdr = embedded_payload(wrb); > struct be_cmd_resp_hdr *resp_hdr; > > - be_dws_le_to_cpu(compl, 4); > + /** > + * To check if valid bit is set, check the entire word as we don't know > + * the endianness of the data (old entry is host endian while a new > + * entry is little endian) > + */ > + if (!compl->flags) { > + beiscsi_log(phba, KERN_ERR, > + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > + "BC_%d : BMBX busy, no completion\n"); > + return -EBUSY; > + } > + compl->flags = le32_to_cpu(compl->flags); > + WARN_ON((compl->flags & CQE_FLAGS_VALID_MASK) == 0); > > + /** > + * Just swap the status to host endian; > + * mcc tag is opaquely copied from mcc_wrb. > + */ > + be_dws_le_to_cpu(compl, 4); > compl_status = (compl->status >> CQE_STATUS_COMPL_SHIFT) & > - CQE_STATUS_COMPL_MASK; > - if (compl_status != MCC_STATUS_SUCCESS) { > - extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & > - CQE_STATUS_EXTD_MASK; > + CQE_STATUS_COMPL_MASK; > + extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & > + CQE_STATUS_EXTD_MASK; > + /* Need to reset the entire word that houses the valid bit */ > + compl->flags = 0; > > - beiscsi_log(phba, KERN_ERR, > - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC_%d : error in cmd completion: " > - "Subsystem : %d Opcode : %d " > - "status(compl/extd)=%d/%d\n", > - hdr->subsystem, hdr->opcode, > - compl_status, extd_status); > - > - if (compl_status == MCC_STATUS_INSUFFICIENT_BUFFER) { > - resp_hdr = (struct be_cmd_resp_hdr *) hdr; > - if (resp_hdr->response_length) > - return 0; > - } > - return -EINVAL; > + if (compl_status == MCC_STATUS_SUCCESS) > + return 0; > + > + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > + "BC_%d : error in cmd completion: Subsystem : %d Opcode : > %d status(compl/extd)=%d/%d\n", > + hdr->subsystem, hdr->opcode, compl_status, extd_status); > + > + if (compl_status == MCC_STATUS_INSUFFICIENT_BUFFER) { > + /* if status is insufficient buffer, check the length */ > + resp_hdr = (struct be_cmd_resp_hdr *) hdr; > + if (resp_hdr->response_length) > + return 0; > } > - return 0; > + return -EINVAL; > } > > static void beiscsi_process_async_link(struct beiscsi_hba *phba, > @@ -453,10 +454,6 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, > struct be_dma_mem *tag_mem; > unsigned int tag, wrb_idx; > > - /** > - * Just swap the status to host endian; mcc tag is opaquely copied > - * from mcc_wrb > - */ > be_dws_le_to_cpu(compl, 4); > tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); > wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn St
Re: [PATCH v6 07/15] scsi: ufs: set REQUEST_SENSE command size to 18 bytes
> On 10/28/2015 02:13 PM, Yaniv Gardi wrote: >> According to UFS device specification REQUEST_SENSE command can >> only report back up to 18 bytes of data. >> >> Reviewed-by: Dolev Raviv >> Signed-off-by: Gilad Broner >> Signed-off-by: Yaniv Gardi >> > Really? The spec only says that the inline sense code is 18 bytes; > if you issue a request sense directly there is not such limitation. > thanks Hannes, so for now, i will exclude this patch from the upcoming V7 regards, Yaniv > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > h...@suse.de+49 911 74053 688 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/12] be2iscsi: Fix be_mcc_compl_poll to use tag_state
On Mon, Feb 01, 2016 at 03:42:45PM +0530, Jitendra Bhivare wrote: > be_mcc_compl_poll waits till 'used' count of MCC WRBQ is zero. This is to > determine the completion of an MCC sent. > > Change function to poll for the tag of MCC sent, instead, and wait till > its tag_state is cleared. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_cmds.c | 92 > + > 1 file changed, 47 insertions(+), 45 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index 12b60dd..60db2de 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -104,19 +104,6 @@ int be_chk_reset_complete(struct beiscsi_hba *phba) > return 0; > } > > -void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag) > -{ > - struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; > - u32 val = 0; > - > - set_bit(MCC_TAG_STATE_RUNNING, &phba->ctrl.ptag_state[tag].tag_state); > - val |= mccq->id & DB_MCCQ_RING_ID_MASK; > - val |= 1 << DB_MCCQ_NUM_POSTED_SHIFT; > - /* ring doorbell after all of request and state is written */ > - wmb(); > - iowrite32(val, phba->db_va + DB_MCCQ_OFFSET); > -} > - > unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > { > unsigned int tag = 0; > @@ -139,6 +126,28 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > return tag; > } > > +void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) > +{ > + spin_lock_bh(&ctrl->mcc_lock); > + tag = tag & MCC_Q_CMD_TAG_MASK; > + ctrl->mcc_tag[ctrl->mcc_free_index] = tag; > + if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) > + ctrl->mcc_free_index = 0; > + else > + ctrl->mcc_free_index++; > + ctrl->mcc_tag_available++; > + spin_unlock_bh(&ctrl->mcc_lock); > +} > + > +/** > + * beiscsi_fail_session(): Closing session with appropriate error > + * @cls_session: ptr to session > + **/ > +void beiscsi_fail_session(struct iscsi_cls_session *cls_session) > +{ > + iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED); > +} > + > /* > * beiscsi_mccq_compl_wait()- Process completion in MCC CQ > * @phba: Driver private structure > @@ -254,19 +263,6 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, > return rc; > } > > -void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) > -{ > - spin_lock(&ctrl->mcc_lock); > - tag = tag & MCC_Q_CMD_TAG_MASK; > - ctrl->mcc_tag[ctrl->mcc_free_index] = tag; > - if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) > - ctrl->mcc_free_index = 0; > - else > - ctrl->mcc_free_index++; > - ctrl->mcc_tag_available++; > - spin_unlock(&ctrl->mcc_lock); > -} > - > static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl) > { > if (compl->flags != 0) { > @@ -328,15 +324,6 @@ static int beiscsi_process_mbox_compl(struct > be_ctrl_info *ctrl, > return 0; > } > > -/** > - * beiscsi_fail_session(): Closing session with appropriate error > - * @cls_session: ptr to session > - **/ > -void beiscsi_fail_session(struct iscsi_cls_session *cls_session) > -{ > - iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED); > -} > - > static void beiscsi_process_async_link(struct beiscsi_hba *phba, > struct be_mcc_compl *compl) > { > @@ -532,6 +519,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, > **/ > int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned int tag) > { > + struct be_ctrl_info *ctrl = &phba->ctrl; > int i; > > for (i = 0; i < mcc_timeout; i++) { > @@ -540,19 +528,33 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, > unsigned int tag) > > beiscsi_process_mcc_cq(phba); > > - if (atomic_read(&phba->ctrl.mcc_obj.q.used) == 0) > + if (!test_bit(MCC_TAG_STATE_RUNNING, > + &ctrl->ptag_state[tag].tag_state)) > break; > udelay(100); > } > - if (i == mcc_timeout) { > - beiscsi_log(phba, KERN_ERR, > - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC_%d : FW Timed Out\n"); > - phba->fw_timeout = true; > - beiscsi_ue_detect(phba); > - return -EBUSY; > - } > - return 0; > + > + if (i < mcc_timeout) > + return 0; > + > + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > + "BC_%d : FW Timed Out\n"); > + phba->fw_timeout = true; > + beiscsi_ue_detect(phba); > + return -EBUSY; > +} > + > +void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag) > +{ > + struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; > + u32 val = 0; > + > + set_bit(MCC_TAG_STATE_RUNNING, &phba->ctrl.ptag_state[tag].tag_state); > + val |= mccq->id & DB
Re: [PATCH 05/12] be2iscsi: Remove be_mbox_notify_wait function
On Mon, Feb 01, 2016 at 03:42:44PM +0530, Jitendra Bhivare wrote: > be_mbox_notify_wait does exactly same thing as be_mbox_notify. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_cmds.c | 79 > +++-- > 1 file changed, 4 insertions(+), 75 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index 8dd8521..12b60dd 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -625,8 +625,6 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET; > struct be_dma_mem *mbox_mem = &ctrl->mbox_mem; > struct be_mcc_mailbox *mbox = mbox_mem->va; > - struct be_mcc_compl *compl = &mbox->compl; > - struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); > > status = be_mbox_db_ready_poll(ctrl); > if (status) > @@ -654,77 +652,8 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > /* RDY is set; small delay before CQE read. */ > udelay(1); > > - if (be_mcc_compl_is_new(compl)) { > - status = beiscsi_process_mbox_compl(ctrl, compl); > - be_mcc_compl_use(compl); > - if (status) { > - beiscsi_log(phba, KERN_ERR, > - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC_%d : After be_mcc_compl_process\n"); > - > - return status; > - } > - } else { > - beiscsi_log(phba, KERN_ERR, > - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC_%d : Invalid Mailbox Completion\n"); > - > - return -EBUSY; > - } > - return 0; > -} > - > -/* > - * Insert the mailbox address into the doorbell in two steps > - * Polls on the mbox doorbell till a command completion (or a timeout) occurs > - */ > -static int be_mbox_notify_wait(struct beiscsi_hba *phba) > -{ > - int status; > - u32 val = 0; > - void __iomem *db = phba->ctrl.db + MPU_MAILBOX_DB_OFFSET; > - struct be_dma_mem *mbox_mem = &phba->ctrl.mbox_mem; > - struct be_mcc_mailbox *mbox = mbox_mem->va; > - struct be_mcc_compl *compl = &mbox->compl; > - struct be_ctrl_info *ctrl = &phba->ctrl; > - > - status = be_mbox_db_ready_poll(ctrl); > - if (status) > - return status; > - > - val |= MPU_MAILBOX_DB_HI_MASK; > - /* at bits 2 - 31 place mbox dma addr msb bits 34 - 63 */ > - val |= (upper_32_bits(mbox_mem->dma) >> 2) << 2; > - iowrite32(val, db); > - > - /* wait for ready to be set */ > - status = be_mbox_db_ready_poll(ctrl); > - if (status != 0) > - return status; > - > - val = 0; > - /* at bits 2 - 31 place mbox dma addr lsb bits 4 - 33 */ > - val |= (u32)(mbox_mem->dma >> 4) << 2; > - iowrite32(val, db); > - > - status = be_mbox_db_ready_poll(ctrl); > - if (status != 0) > - return status; > - > - /* A cq entry has been made now */ > - if (be_mcc_compl_is_new(compl)) { > - status = be_mcc_compl_process(ctrl, &mbox->compl); > - be_mcc_compl_use(compl); > - if (status) > - return status; > - } else { > - beiscsi_log(phba, KERN_ERR, > - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC_%d : invalid mailbox completion\n"); > - > - return -EBUSY; > - } > - return 0; > + status = beiscsi_process_mbox_compl(ctrl, &mbox->compl); > + return status; > } > > void be_wrb_hdr_prepare(struct be_mcc_wrb *wrb, int payload_len, > @@ -1039,7 +968,7 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba, > > be_cmd_page_addrs_prepare(req->pages, ARRAY_SIZE(req->pages), q_mem); > > - status = be_mbox_notify_wait(phba); > + status = be_mbox_notify(ctrl); > if (!status) { > struct be_cmd_resp_mcc_create *resp = embedded_payload(wrb); > mccq->id = le16_to_cpu(resp->id); > @@ -1381,7 +1310,7 @@ int beiscsi_cmd_reset_function(struct beiscsi_hba > *phba) > be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0); > be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON, > OPCODE_COMMON_FUNCTION_RESET, sizeof(*req)); > - status = be_mbox_notify_wait(phba); > + status = be_mbox_notify(ctrl); > > mutex_unlock(&ctrl->mbox_lock); > return status; > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Ime
Re: [PATCH 04/12] be2iscsi: Rename MCC and BMBX processing functions
On Mon, Feb 01, 2016 at 03:42:43PM +0530, Jitendra Bhivare wrote: > beiscsi_mccq_compl -> beiscsi_mccq_compl_wait - indicate blocking call. > be_mcc_wait_compl -> be_mcc_compl_poll - indicate polling for completion. > be_mbox_db_ready_wait -> be_mbox_db_ready_poll - indicate polling for RDY. > be_mcc_compl_process -> beiscsi_process_mbox_compl - indicate BMBX compl. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_cmds.c | 35 +-- > drivers/scsi/be2iscsi/be_cmds.h | 6 +++--- > drivers/scsi/be2iscsi/be_iscsi.c | 8 > drivers/scsi/be2iscsi/be_main.c | 8 > drivers/scsi/be2iscsi/be_mgmt.c | 12 ++-- > 5 files changed, 34 insertions(+), 35 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index fa010ac..8dd8521 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -140,7 +140,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > } > > /* > - * beiscsi_mccq_compl()- Wait for completion of MBX > + * beiscsi_mccq_compl_wait()- Process completion in MCC CQ > * @phba: Driver private structure > * @tag: Tag for the MBX Command > * @wrb: the WRB used for the MBX Command > @@ -152,9 +152,9 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > * Success: 0 > * Failure: Non-Zero > **/ > -int beiscsi_mccq_compl(struct beiscsi_hba *phba, > - uint32_t tag, struct be_mcc_wrb **wrb, > - struct be_dma_mem *mbx_cmd_mem) > +int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, > + uint32_t tag, struct be_mcc_wrb **wrb, > + struct be_dma_mem *mbx_cmd_mem) > { > int rc = 0; > uint32_t mcc_tag_status; > @@ -283,7 +283,7 @@ static inline void be_mcc_compl_use(struct be_mcc_compl > *compl) > } > > /* > - * be_mcc_compl_process()- Check the MBX comapletion status > + * beiscsi_process_mbox_compl()- Check the MBX completion status > * @ctrl: Function specific MBX data structure > * @compl: Completion status of MBX Command > * > @@ -293,8 +293,8 @@ static inline void be_mcc_compl_use(struct be_mcc_compl > *compl) > * Success: Zero > * Failure: Non-Zero > **/ > -static int be_mcc_compl_process(struct be_ctrl_info *ctrl, > - struct be_mcc_compl *compl) > +static int beiscsi_process_mbox_compl(struct be_ctrl_info *ctrl, > + struct be_mcc_compl *compl) > { > u16 compl_status, extd_status; > struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem); > @@ -520,7 +520,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, > } > > /* > - * be_mcc_wait_compl()- Wait for MBX completion > + * be_mcc_compl_poll()- Wait for MBX completion > * @phba: driver private structure > * > * Wait till no more pending mcc requests are present > @@ -556,8 +556,7 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned > int tag) > } > > /* > -/* > - * be_mbox_db_ready_wait()- Check ready status > + * be_mbox_db_ready_poll()- Check ready status > * @ctrl: Function specific MBX data structure > * > * Check for the ready status of FW to send BMBX > @@ -567,7 +566,7 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned > int tag) > * Success: 0 > * Failure: Non-Zero > **/ > -static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl) > +static int be_mbox_db_ready_poll(struct be_ctrl_info *ctrl) > { > /* wait 30s for generic non-flash MBOX operation */ > #define BEISCSI_MBX_RDY_BIT_TIMEOUT 3 > @@ -629,7 +628,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > struct be_mcc_compl *compl = &mbox->compl; > struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); > > - status = be_mbox_db_ready_wait(ctrl); > + status = be_mbox_db_ready_poll(ctrl); > if (status) > return status; > > @@ -638,7 +637,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > val |= (upper_32_bits(mbox_mem->dma) >> 2) << 2; > iowrite32(val, db); > > - status = be_mbox_db_ready_wait(ctrl); > + status = be_mbox_db_ready_poll(ctrl); > if (status) > return status; > > @@ -648,7 +647,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > val |= (u32) (mbox_mem->dma >> 4) << 2; > iowrite32(val, db); > > - status = be_mbox_db_ready_wait(ctrl); > + status = be_mbox_db_ready_poll(ctrl); > if (status) > return status; > > @@ -656,7 +655,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) > udelay(1); > > if (be_mcc_compl_is_new(compl)) { > - status = be_mcc_compl_process(ctrl, &mbox->compl); > + status = beiscsi_process_mbox_compl(ctrl, compl); > be_mcc_compl_use(compl); > if (status) { > beiscsi_log(phba, KERN_ERR, > @@ -689,7 +688,7 @@ static int be_mbox_notify_wa
Re: [PATCH 03/12] be2iscsi: Remove redundant MCC processing code
On Mon, Feb 01, 2016 at 03:42:42PM +0530, Jitendra Bhivare wrote: > be_mcc_compl_process_isr is removed. > MCC CQ processing is done only in beiscsi_process_mcc_cq and MCC CQE > processing is done only in beiscsi_process_mcc_compl. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be_cmds.c | 164 > ++-- > drivers/scsi/be2iscsi/be_cmds.h | 7 +- > drivers/scsi/be2iscsi/be_main.c | 8 +- > drivers/scsi/be2iscsi/be_main.h | 1 + > drivers/scsi/be2iscsi/be_mgmt.c | 3 +- > 5 files changed, 68 insertions(+), 115 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index c5e7739..fa010ac 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -328,76 +328,6 @@ static int be_mcc_compl_process(struct be_ctrl_info > *ctrl, > return 0; > } > > -int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl, > - struct be_mcc_compl *compl) > -{ > - struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); > - u16 compl_status, extd_status; > - struct be_dma_mem *tag_mem; > - unsigned int tag, wrb_idx; > - > - be_dws_le_to_cpu(compl, 4); > - tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); > - wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; > - > - if (!test_bit(MCC_TAG_STATE_RUNNING, > - &ctrl->ptag_state[tag].tag_state)) { > - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_MBOX | > - BEISCSI_LOG_INIT | BEISCSI_LOG_CONFIG, > - "BC_%d : MBX cmd completed but not posted\n"); > - return 0; > - } > - > - if (test_bit(MCC_TAG_STATE_TIMEOUT, > - &ctrl->ptag_state[tag].tag_state)) { > - beiscsi_log(phba, KERN_WARNING, > - BEISCSI_LOG_MBOX | BEISCSI_LOG_INIT | > - BEISCSI_LOG_CONFIG, > - "BC_%d : MBX Completion for timeout Command from > FW\n"); > - /** > - * Check for the size before freeing resource. > - * Only for non-embedded cmd, PCI resource is allocated. > - **/ > - tag_mem = &ctrl->ptag_state[tag].tag_mem_state; > - if (tag_mem->size) > - pci_free_consistent(ctrl->pdev, tag_mem->size, > - tag_mem->va, tag_mem->dma); > - free_mcc_tag(ctrl, tag); > - return 0; > - } > - > - compl_status = (compl->status >> CQE_STATUS_COMPL_SHIFT) & > -CQE_STATUS_COMPL_MASK; > - extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & > - CQE_STATUS_EXTD_MASK; > - /* The ctrl.mcc_tag_status[tag] is filled with > - * [31] = valid, [30:24] = Rsvd, [23:16] = wrb, [15:8] = extd_status, > - * [7:0] = compl_status > - */ > - ctrl->mcc_tag_status[tag] = CQE_VALID_MASK; > - ctrl->mcc_tag_status[tag] |= (wrb_idx << CQE_STATUS_WRB_SHIFT); > - ctrl->mcc_tag_status[tag] |= (extd_status << CQE_STATUS_ADDL_SHIFT) & > - CQE_STATUS_ADDL_MASK; > - ctrl->mcc_tag_status[tag] |= (compl_status & CQE_STATUS_MASK); > - > - /* write ordering implied in wake_up_interruptible */ > - clear_bit(MCC_TAG_STATE_RUNNING, &ctrl->ptag_state[tag].tag_state); > - wake_up_interruptible(&ctrl->mcc_wait[tag]); > - return 0; > -} > - > -static struct be_mcc_compl *be_mcc_compl_get(struct beiscsi_hba *phba) > -{ > - struct be_queue_info *mcc_cq = &phba->ctrl.mcc_obj.cq; > - struct be_mcc_compl *compl = queue_tail_node(mcc_cq); > - > - if (be_mcc_compl_is_new(compl)) { > - queue_tail_inc(mcc_cq); > - return compl; > - } > - return NULL; > -} > - > /** > * beiscsi_fail_session(): Closing session with appropriate error > * @cls_session: ptr to session > @@ -528,27 +458,65 @@ void beiscsi_process_async_event(struct beiscsi_hba > *phba, > evt_code, compl->status, compl->flags); > } > > -int beiscsi_process_mcc(struct beiscsi_hba *phba) > +int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, > + struct be_mcc_compl *compl) > { > - struct be_mcc_compl *compl; > - int num = 0, status = 0; > - struct be_ctrl_info *ctrl = &phba->ctrl; > + struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); > + u16 compl_status, extd_status; > + struct be_dma_mem *tag_mem; > + unsigned int tag, wrb_idx; > > - while ((compl = be_mcc_compl_get(phba))) { > - if (compl->flags & CQE_FLAGS_ASYNC_MASK) { > - beiscsi_process_async_event(phba, compl); > - } else if (compl->flags & CQE_FLAGS_COMPLETED_MASK) { > - status = be_mcc_compl_process(ctrl, compl); > - atomic_dec(&phba->ctrl.mcc_obj.q.used);
Re: [PATCH 01/12] be2iscsi: Remove unused mcc_cq_lock
On Mon, 2016-02-01 at 15:42 +0530, Jitendra Bhivare wrote: > mcc_cq_lock spin_lock is used only in beiscsi_process_mcc which is > called > only when all interrupts are disabled from mgmt_epfw_cleanup during > unloading of driver. There is no other context where there can be > contention for the processing of CQ. Removing a lock is not a bug fix unless it's causing a user visible problem, so this patch (and quite a lot of others in this series) should go through the merge window process. For things that cause user visible problems, we need a description of the problem in the changelog and a cc to stable unless it was a regression in the 4.4+ merge window. Thanks, James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/12] be2iscsi: Use macros for MCC WRB and CQE fields
On Mon, Feb 01, 2016 at 03:42:41PM +0530, Jitendra Bhivare wrote: > Rename mcc_numtag to mcc_tag_status. > MCC CQE status is processed using macros already defined in be_cmds.h. > > Add MCC_Q_WRB_ and MCC_Q_CMD_TAG_MASK macros to map to already defined > CQE_STATUS_ macros to be consistent when posting MCC. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be.h | 8 +++- > drivers/scsi/be2iscsi/be_cmds.c | 40 +--- > drivers/scsi/be2iscsi/be_cmds.h | 13 +++-- > drivers/scsi/be2iscsi/be_main.c | 11 ++- > 4 files changed, 41 insertions(+), 31 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h > index 1524fe4..da1d87a 100644 > --- a/drivers/scsi/be2iscsi/be.h > +++ b/drivers/scsi/be2iscsi/be.h > @@ -135,7 +135,7 @@ struct be_ctrl_info { > > wait_queue_head_t mcc_wait[MAX_MCC_CMD + 1]; > unsigned int mcc_tag[MAX_MCC_CMD]; > - unsigned int mcc_numtag[MAX_MCC_CMD + 1]; > + unsigned int mcc_tag_status[MAX_MCC_CMD + 1]; > unsigned short mcc_alloc_index; > unsigned short mcc_free_index; > unsigned int mcc_tag_available; > @@ -145,6 +145,12 @@ struct be_ctrl_info { > > #include "be_cmds.h" > > +/* WRB index mask for MCC_Q_LEN queue entries */ > +#define MCC_Q_WRB_IDX_MASK CQE_STATUS_WRB_MASK > +#define MCC_Q_WRB_IDX_SHIFT CQE_STATUS_WRB_SHIFT > +/* TAG is from 1...MAX_MCC_CMD, MASK includes MAX_MCC_CMD */ > +#define MCC_Q_CMD_TAG_MASK ((MAX_MCC_CMD << 1) - 1) > + > #define PAGE_SHIFT_4K 12 > #define PAGE_SIZE_4K (1 << PAGE_SHIFT_4K) > #define mcc_timeout 12 /* 12s timeout */ > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index e8e9d22..c5e7739 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -125,7 +125,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) > if (phba->ctrl.mcc_tag_available) { > tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index]; > phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0; > - phba->ctrl.mcc_numtag[tag] = 0; > + phba->ctrl.mcc_tag_status[tag] = 0; > phba->ctrl.ptag_state[tag].tag_state = 0; > } > if (tag) { > @@ -157,7 +157,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, > struct be_dma_mem *mbx_cmd_mem) > { > int rc = 0; > - uint32_t mcc_tag_response; > + uint32_t mcc_tag_status; > uint16_t status = 0, addl_status = 0, wrb_num = 0; > struct be_mcc_wrb *temp_wrb; > struct be_cmd_req_hdr *mbx_hdr; > @@ -172,7 +172,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, > /* wait for the mccq completion */ > rc = wait_event_interruptible_timeout( > phba->ctrl.mcc_wait[tag], > - phba->ctrl.mcc_numtag[tag], > + phba->ctrl.mcc_tag_status[tag], > msecs_to_jiffies( > BEISCSI_HOST_MBX_TIMEOUT)); > /** > @@ -209,15 +209,15 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, > } > > rc = 0; > - mcc_tag_response = phba->ctrl.mcc_numtag[tag]; > - status = (mcc_tag_response & CQE_STATUS_MASK); > - addl_status = ((mcc_tag_response & CQE_STATUS_ADDL_MASK) >> > + mcc_tag_status = phba->ctrl.mcc_tag_status[tag]; > + status = (mcc_tag_status & CQE_STATUS_MASK); > + addl_status = ((mcc_tag_status & CQE_STATUS_ADDL_MASK) >> > CQE_STATUS_ADDL_SHIFT); > > if (mbx_cmd_mem) { > mbx_hdr = (struct be_cmd_req_hdr *)mbx_cmd_mem->va; > } else { > - wrb_num = (mcc_tag_response & CQE_STATUS_WRB_MASK) >> > + wrb_num = (mcc_tag_status & CQE_STATUS_WRB_MASK) >> > CQE_STATUS_WRB_SHIFT; > temp_wrb = (struct be_mcc_wrb *)queue_get_wrb(mccq, wrb_num); > mbx_hdr = embedded_payload(temp_wrb); > @@ -257,7 +257,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, > void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) > { > spin_lock(&ctrl->mcc_lock); > - tag = tag & 0x00FF; > + tag = tag & MCC_Q_CMD_TAG_MASK; > ctrl->mcc_tag[ctrl->mcc_free_index] = tag; > if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) > ctrl->mcc_free_index = 0; > @@ -334,10 +334,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl, > struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); > u16 compl_status, extd_status; > struct be_dma_mem *tag_mem; > - unsigned short tag; > + unsigned int tag, wrb_idx; > > be_dws_le_to_cpu(compl, 4); > - tag = (compl->tag0 & 0x00FF); > + tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); > + wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; > > if (!test_bit(MCC_TAG_STATE_RU
Re: [PATCH 01/12] be2iscsi: Remove unused mcc_cq_lock
On Mon, Feb 01, 2016 at 03:42:40PM +0530, Jitendra Bhivare wrote: > mcc_cq_lock spin_lock is used only in beiscsi_process_mcc which is called > only when all interrupts are disabled from mgmt_epfw_cleanup during > unloading of driver. There is no other context where there can be > contention for the processing of CQ. > > Signed-off-by: Jitendra Bhivare > --- > drivers/scsi/be2iscsi/be.h | 1 - > drivers/scsi/be2iscsi/be_cmds.c | 2 -- > drivers/scsi/be2iscsi/be_main.c | 1 - > 3 files changed, 4 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h > index 7d425af..1524fe4 100644 > --- a/drivers/scsi/be2iscsi/be.h > +++ b/drivers/scsi/be2iscsi/be.h > @@ -132,7 +132,6 @@ struct be_ctrl_info { > /* MCC Rings */ > struct be_mcc_obj mcc_obj; > spinlock_t mcc_lock;/* For serializing mcc cmds to BE card */ > - spinlock_t mcc_cq_lock; > > wait_queue_head_t mcc_wait[MAX_MCC_CMD + 1]; > unsigned int mcc_tag[MAX_MCC_CMD]; > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index 34c33d4..e8e9d22 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -532,7 +532,6 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba) > int num = 0, status = 0; > struct be_ctrl_info *ctrl = &phba->ctrl; > > - spin_lock_bh(&phba->ctrl.mcc_cq_lock); > while ((compl = be_mcc_compl_get(phba))) { > if (compl->flags & CQE_FLAGS_ASYNC_MASK) { > beiscsi_process_async_event(phba, compl); > @@ -547,7 +546,6 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba) > if (num) > hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1); > > - spin_unlock_bh(&phba->ctrl.mcc_cq_lock); > return status; > } > > diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c > index 70179e1..314fd2c 100644 > --- a/drivers/scsi/be2iscsi/be_main.c > +++ b/drivers/scsi/be2iscsi/be_main.c > @@ -730,7 +730,6 @@ static int be_ctrl_init(struct beiscsi_hba *phba, struct > pci_dev *pdev) > memset(mbox_mem_align->va, 0, sizeof(struct be_mcc_mailbox)); > mutex_init(&ctrl->mbox_lock); > spin_lock_init(&phba->ctrl.mcc_lock); > - spin_lock_init(&phba->ctrl.mcc_cq_lock); > > return status; > } > -- > 2.5.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Reviewed-by: Johannes Thumshirn -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re: lpfc: Modularize and cleanup FDMI code in driver
Hello James Smart, The patch 4258e98ee386: "lpfc: Modularize and cleanup FDMI code in driver" from Dec 16, 2015, leads to the following static checker warning: drivers/scsi/lpfc/lpfc_ct.c:2547 lpfc_fdmi_cmd() error: __memcpy() '&rh->rpl.pe' too small (4 vs 8) drivers/scsi/lpfc/lpfc_ct.c 2534 /* Next fill in the specific FDMI cmd information */ 2535 switch (cmdcode) { 2536 case SLI_MGMT_RHAT: 2537 case SLI_MGMT_RHBA: 2538 rh = (struct lpfc_fdmi_reg_hba *)&CtReq->un.PortID; 2539 /* HBA Identifier */ 2540 memcpy(&rh->hi.PortName, &phba->pport->fc_sparam.portName, 2541 sizeof(struct lpfc_name)); 2542 2543 if (cmdcode == SLI_MGMT_RHBA) { 2544 /* Registered Port List */ 2545 /* One entry (port) per adapter */ 2546 rh->rpl.EntryCnt = cpu_to_be32(1); 2547 memcpy(&rh->rpl.pe, &phba->pport->fc_sparam.portName, ^^^ .pe is a u32. 2548 sizeof(struct lpfc_name)); lpfc_name is an 8 byte union. 2549 regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mm: another VM_BUG_ON_PAGE(PageTail(page))
On Fri, Jan 29, 2016 at 1:35 PM, Kirill A. Shutemov wrote: > From 691a961bb401c5815ed741dac63591efbc6027e3 Mon Sep 17 00:00:00 2001 > From: "Kirill A. Shutemov" > Date: Fri, 29 Jan 2016 15:06:17 +0300 > Subject: [PATCH 2/2] mempolicy: do not try to queue pages from > !vma_migratable() > > Maybe I miss some point, but I don't see a reason why we try to queue > pages from non migratable VMAs. > > The only case when we can queue pages from such VMA is MPOL_MF_STRICT > plus MPOL_MF_MOVE or MPOL_MF_MOVE_ALL for VMA which has pages on LRU, > but gfp mask is not sutable for migaration (see mapping_gfp_mask() check > in vma_migratable()). That's looks like a bug to me. > > Let's filter out non-migratable vma at start of queue_pages_test_walk() > and go to queue_pages_pte_range() only if MPOL_MF_MOVE or > MPOL_MF_MOVE_ALL flag is set. I've run the fuzzer with these two patches for the weekend and seen no crashes. I guess we can consider this as fixed. Thanks! > Signed-off-by: Kirill A. Shutemov > --- > mm/mempolicy.c | 14 +- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 27d135408a22..4c4187c0e1de 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -548,8 +548,7 @@ retry: > goto retry; > } > > - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > - migrate_page_add(page, qp->pagelist, flags); > + migrate_page_add(page, qp->pagelist, flags); > } > pte_unmap_unlock(pte - 1, ptl); > cond_resched(); > @@ -625,7 +624,7 @@ static int queue_pages_test_walk(unsigned long start, > unsigned long end, > unsigned long endvma = vma->vm_end; > unsigned long flags = qp->flags; > > - if (vma->vm_flags & VM_PFNMAP) > + if (!vma_migratable(vma)) > return 1; > > if (endvma > end) > @@ -644,16 +643,13 @@ static int queue_pages_test_walk(unsigned long start, > unsigned long end, > > if (flags & MPOL_MF_LAZY) { > /* Similar to task_numa_work, skip inaccessible VMAs */ > - if (vma_migratable(vma) && > - vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)) > + if (vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)) > change_prot_numa(vma, start, endvma); > return 1; > } > > - if ((flags & MPOL_MF_STRICT) || > - ((flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) && > -vma_migratable(vma))) > - /* queue pages from current vma */ > + /* queue pages from current vma */ > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > return 0; > return 1; > } > -- > 2.7.0.rc3 > > -- > You received this message because you are subscribed to the Google Groups > "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to syzkaller+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v6 01/15] scsi: ufs: clear UTRD, UPIU req and rsp before new transfers
> > >> +cdb_len = min_t(unsigned short, lrbp->cmd->cmd_len, MAX_CDB_SIZE); >> +memcpy(ucd_req_ptr->sc.cdb, lrbp->cmd->cmnd, cdb_len); >> +if (cdb_len < MAX_CDB_SIZE) >> +memset(ucd_req_ptr->sc.cdb + cdb_len, 0, >> + (MAX_CDB_SIZE - cdb_len)); > It's just 16 bytes, setting all to zero prior to copy will be as good as > this if statement > memset(ucd_req_ptr->sc.cdb, 0, MAX_CDB_SIZE); > i agree. will be modified in V7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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 linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/12] be2iscsi: critical fixes for 11.0.0.0
This driver update has critical fixes for following issues: - Management tasks with incorrect ExpStatSn - WRB allocation failures in IO path - MCC WRB leak Jitendra Bhivare (12): be2iscsi: Remove unused mcc_cq_lock be2iscsi: Use macros for MCC WRB and CQE fields be2iscsi: Remove redundant MCC processing code be2iscsi: Rename MCC and BMBX processing functions be2iscsi: Remove be_mbox_notify_wait function be2iscsi: Fix be_mcc_compl_poll to use tag_state be2iscsi: Cleanup processing of BMBX completion be2iscsi: Fix MCC WRB leak in open_connection be2iscsi: Couple MCC tag and WRB alloc and free be2iscsi: Fix ExpStatSn in management tasks be2iscsi: _bh for io_sgl_lock and mgmt_sgl_lock be2iscsi: Add lock to protect WRB alloc and free drivers/scsi/be2iscsi/be.h | 11 +- drivers/scsi/be2iscsi/be_cmds.c | 522 +-- drivers/scsi/be2iscsi/be_cmds.h | 32 +-- drivers/scsi/be2iscsi/be_iscsi.c | 8 +- drivers/scsi/be2iscsi/be_main.c | 73 +++--- drivers/scsi/be2iscsi/be_main.h | 2 + drivers/scsi/be2iscsi/be_mgmt.c | 167 + 7 files changed, 355 insertions(+), 460 deletions(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/12] be2iscsi: Couple MCC tag and WRB alloc and free
WARN_ON(atomic_read(&mccq->used) >= mccq->len) seen when FW gets into UE. MCCQ overflow is happening because driver discards any new request and frees up the tag. The tag allocation controls the number of MCC WRB posted. It is being replenished but WRBs are not hence the WARN_ON. Allocation and freeing of WRB and tags for MCC is now done in one place. This helps to achieve proper accounting of WRB indices and MCC tags. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be.h | 2 +- drivers/scsi/be2iscsi/be_cmds.c | 103 +- drivers/scsi/be2iscsi/be_cmds.h | 6 +- drivers/scsi/be2iscsi/be_main.c | 3 +- drivers/scsi/be2iscsi/be_mgmt.c | 134 +++- 5 files changed, 130 insertions(+), 118 deletions(-) diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h index da1d87a..ee5ace8 100644 --- a/drivers/scsi/be2iscsi/be.h +++ b/drivers/scsi/be2iscsi/be.h @@ -42,7 +42,7 @@ struct be_queue_info { u16 id; u16 tail, head; bool created; - atomic_t used; /* Number of valid elements in the queue */ + u16 used; /* Number of valid elements in the queue */ }; static inline u32 MODULO(u16 val, u16 limit) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index 728aa133..a55eaee 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -126,8 +126,62 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) return tag; } -void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) +struct be_mcc_wrb *alloc_mcc_wrb(struct beiscsi_hba *phba, +unsigned int *ref_tag) { + struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; + struct be_mcc_wrb *wrb = NULL; + unsigned int tag; + + spin_lock_bh(&phba->ctrl.mcc_lock); + if (mccq->used == mccq->len) { + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, + "BC_%d : MCC queue full: WRB used %u tag avail %u\n", + mccq->used, phba->ctrl.mcc_tag_available); + goto alloc_failed; + } + + if (!phba->ctrl.mcc_tag_available) + goto alloc_failed; + + tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index]; + if (!tag) { + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, + "BC_%d : MCC tag 0 allocated: tag avail %u alloc index %u\n", + phba->ctrl.mcc_tag_available, + phba->ctrl.mcc_alloc_index); + goto alloc_failed; + } + + /* return this tag for further reference */ + *ref_tag = tag; + phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0; + phba->ctrl.mcc_tag_status[tag] = 0; + phba->ctrl.ptag_state[tag].tag_state = 0; + phba->ctrl.mcc_tag_available--; + if (phba->ctrl.mcc_alloc_index == (MAX_MCC_CMD - 1)) + phba->ctrl.mcc_alloc_index = 0; + else + phba->ctrl.mcc_alloc_index++; + + wrb = queue_head_node(mccq); + memset(wrb, 0, sizeof(*wrb)); + wrb->tag0 = tag; + wrb->tag0 |= (mccq->head << MCC_Q_WRB_IDX_SHIFT) & MCC_Q_WRB_IDX_MASK; + queue_head_inc(mccq); + mccq->used++; + +alloc_failed: + spin_unlock_bh(&phba->ctrl.mcc_lock); + return wrb; +} + +void free_mcc_wrb(struct be_ctrl_info *ctrl, unsigned int tag) +{ + struct be_queue_info *mccq = &ctrl->mcc_obj.q; + spin_lock_bh(&ctrl->mcc_lock); tag = tag & MCC_Q_CMD_TAG_MASK; ctrl->mcc_tag[ctrl->mcc_free_index] = tag; @@ -136,6 +190,7 @@ void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) else ctrl->mcc_free_index++; ctrl->mcc_tag_available++; + mccq->used--; spin_unlock_bh(&ctrl->mcc_lock); } @@ -173,10 +228,8 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, struct be_cmd_resp_hdr *mbx_resp_hdr; struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; - if (beiscsi_error(phba)) { - free_mcc_tag(&phba->ctrl, tag); + if (beiscsi_error(phba)) return -EPERM; - } /* wait for the mccq completion */ rc = wait_event_interruptible_timeout( @@ -259,7 +312,7 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, } } - free_mcc_tag(&phba->ctrl, tag); + free_mcc_wrb(&phba->ctrl, tag); return rc; } @@ -479,7 +532,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, if (tag_mem->size) pci_free_consistent(ctrl->pdev, tag_mem->size, tag_mem->va, tag_mem->dma); - free_mcc_tag(ctrl,
[PATCH 05/12] be2iscsi: Remove be_mbox_notify_wait function
be_mbox_notify_wait does exactly same thing as be_mbox_notify. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_cmds.c | 79 +++-- 1 file changed, 4 insertions(+), 75 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index 8dd8521..12b60dd 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -625,8 +625,6 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) void __iomem *db = ctrl->db + MPU_MAILBOX_DB_OFFSET; struct be_dma_mem *mbox_mem = &ctrl->mbox_mem; struct be_mcc_mailbox *mbox = mbox_mem->va; - struct be_mcc_compl *compl = &mbox->compl; - struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); status = be_mbox_db_ready_poll(ctrl); if (status) @@ -654,77 +652,8 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) /* RDY is set; small delay before CQE read. */ udelay(1); - if (be_mcc_compl_is_new(compl)) { - status = beiscsi_process_mbox_compl(ctrl, compl); - be_mcc_compl_use(compl); - if (status) { - beiscsi_log(phba, KERN_ERR, - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, - "BC_%d : After be_mcc_compl_process\n"); - - return status; - } - } else { - beiscsi_log(phba, KERN_ERR, - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, - "BC_%d : Invalid Mailbox Completion\n"); - - return -EBUSY; - } - return 0; -} - -/* - * Insert the mailbox address into the doorbell in two steps - * Polls on the mbox doorbell till a command completion (or a timeout) occurs - */ -static int be_mbox_notify_wait(struct beiscsi_hba *phba) -{ - int status; - u32 val = 0; - void __iomem *db = phba->ctrl.db + MPU_MAILBOX_DB_OFFSET; - struct be_dma_mem *mbox_mem = &phba->ctrl.mbox_mem; - struct be_mcc_mailbox *mbox = mbox_mem->va; - struct be_mcc_compl *compl = &mbox->compl; - struct be_ctrl_info *ctrl = &phba->ctrl; - - status = be_mbox_db_ready_poll(ctrl); - if (status) - return status; - - val |= MPU_MAILBOX_DB_HI_MASK; - /* at bits 2 - 31 place mbox dma addr msb bits 34 - 63 */ - val |= (upper_32_bits(mbox_mem->dma) >> 2) << 2; - iowrite32(val, db); - - /* wait for ready to be set */ - status = be_mbox_db_ready_poll(ctrl); - if (status != 0) - return status; - - val = 0; - /* at bits 2 - 31 place mbox dma addr lsb bits 4 - 33 */ - val |= (u32)(mbox_mem->dma >> 4) << 2; - iowrite32(val, db); - - status = be_mbox_db_ready_poll(ctrl); - if (status != 0) - return status; - - /* A cq entry has been made now */ - if (be_mcc_compl_is_new(compl)) { - status = be_mcc_compl_process(ctrl, &mbox->compl); - be_mcc_compl_use(compl); - if (status) - return status; - } else { - beiscsi_log(phba, KERN_ERR, - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, - "BC_%d : invalid mailbox completion\n"); - - return -EBUSY; - } - return 0; + status = beiscsi_process_mbox_compl(ctrl, &mbox->compl); + return status; } void be_wrb_hdr_prepare(struct be_mcc_wrb *wrb, int payload_len, @@ -1039,7 +968,7 @@ int beiscsi_cmd_mccq_create(struct beiscsi_hba *phba, be_cmd_page_addrs_prepare(req->pages, ARRAY_SIZE(req->pages), q_mem); - status = be_mbox_notify_wait(phba); + status = be_mbox_notify(ctrl); if (!status) { struct be_cmd_resp_mcc_create *resp = embedded_payload(wrb); mccq->id = le16_to_cpu(resp->id); @@ -1381,7 +1310,7 @@ int beiscsi_cmd_reset_function(struct beiscsi_hba *phba) be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0); be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON, OPCODE_COMMON_FUNCTION_RESET, sizeof(*req)); - status = be_mbox_notify_wait(phba); + status = be_mbox_notify(ctrl); mutex_unlock(&ctrl->mbox_lock); return status; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/12] be2iscsi: Add lock to protect WRB alloc and free
FW got into UE after running IO stress test With kernel change to split session lock in frwd_lock and back_lock for tx and rx path correspondingly, in the IO path, common resource used in driver such as WRB was left unprotected. Add wrb_lock spinlock to protect allocation and freeing of WRB. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_main.c | 5 + drivers/scsi/be2iscsi/be_main.h | 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index fa2b589..0892ee2 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -1190,12 +1190,14 @@ beiscsi_get_wrb_handle(struct hwi_wrb_context *pwrb_context, { struct wrb_handle *pwrb_handle; + spin_lock_bh(&pwrb_context->wrb_lock); pwrb_handle = pwrb_context->pwrb_handle_base[pwrb_context->alloc_index]; pwrb_context->wrb_handles_available--; if (pwrb_context->alloc_index == (wrbs_per_cxn - 1)) pwrb_context->alloc_index = 0; else pwrb_context->alloc_index++; + spin_unlock_bh(&pwrb_context->wrb_lock); return pwrb_handle; } @@ -1227,12 +1229,14 @@ beiscsi_put_wrb_handle(struct hwi_wrb_context *pwrb_context, struct wrb_handle *pwrb_handle, unsigned int wrbs_per_cxn) { + spin_lock_bh(&pwrb_context->wrb_lock); pwrb_context->pwrb_handle_base[pwrb_context->free_index] = pwrb_handle; pwrb_context->wrb_handles_available++; if (pwrb_context->free_index == (wrbs_per_cxn - 1)) pwrb_context->free_index = 0; else pwrb_context->free_index++; + spin_unlock_bh(&pwrb_context->wrb_lock); } /** @@ -2920,6 +2924,7 @@ static int beiscsi_init_wrb_handle(struct beiscsi_hba *phba) } num_cxn_wrbh--; } + spin_lock_init(&pwrb_context->wrb_lock); } idx = 0; for (index = 0; index < phba->params.cxns_per_ctrl; index++) { diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h index 5ded3fa..30a4606 100644 --- a/drivers/scsi/be2iscsi/be_main.h +++ b/drivers/scsi/be2iscsi/be_main.h @@ -304,6 +304,7 @@ struct invalidate_command_table { #define BEISCSI_GET_ULP_FROM_CRI(phwi_ctrlr, cri) \ (phwi_ctrlr->wrb_context[cri].ulp_num) struct hwi_wrb_context { + spinlock_t wrb_lock; struct list_head wrb_handle_list; struct list_head wrb_handle_drvr_list; struct wrb_handle **pwrb_handle_base; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/12] be2iscsi: Remove redundant MCC processing code
be_mcc_compl_process_isr is removed. MCC CQ processing is done only in beiscsi_process_mcc_cq and MCC CQE processing is done only in beiscsi_process_mcc_compl. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_cmds.c | 164 ++-- drivers/scsi/be2iscsi/be_cmds.h | 7 +- drivers/scsi/be2iscsi/be_main.c | 8 +- drivers/scsi/be2iscsi/be_main.h | 1 + drivers/scsi/be2iscsi/be_mgmt.c | 3 +- 5 files changed, 68 insertions(+), 115 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index c5e7739..fa010ac 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -328,76 +328,6 @@ static int be_mcc_compl_process(struct be_ctrl_info *ctrl, return 0; } -int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl, - struct be_mcc_compl *compl) -{ - struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); - u16 compl_status, extd_status; - struct be_dma_mem *tag_mem; - unsigned int tag, wrb_idx; - - be_dws_le_to_cpu(compl, 4); - tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); - wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; - - if (!test_bit(MCC_TAG_STATE_RUNNING, - &ctrl->ptag_state[tag].tag_state)) { - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_MBOX | - BEISCSI_LOG_INIT | BEISCSI_LOG_CONFIG, - "BC_%d : MBX cmd completed but not posted\n"); - return 0; - } - - if (test_bit(MCC_TAG_STATE_TIMEOUT, -&ctrl->ptag_state[tag].tag_state)) { - beiscsi_log(phba, KERN_WARNING, - BEISCSI_LOG_MBOX | BEISCSI_LOG_INIT | - BEISCSI_LOG_CONFIG, - "BC_%d : MBX Completion for timeout Command from FW\n"); - /** -* Check for the size before freeing resource. -* Only for non-embedded cmd, PCI resource is allocated. -**/ - tag_mem = &ctrl->ptag_state[tag].tag_mem_state; - if (tag_mem->size) - pci_free_consistent(ctrl->pdev, tag_mem->size, - tag_mem->va, tag_mem->dma); - free_mcc_tag(ctrl, tag); - return 0; - } - - compl_status = (compl->status >> CQE_STATUS_COMPL_SHIFT) & - CQE_STATUS_COMPL_MASK; - extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & - CQE_STATUS_EXTD_MASK; - /* The ctrl.mcc_tag_status[tag] is filled with -* [31] = valid, [30:24] = Rsvd, [23:16] = wrb, [15:8] = extd_status, -* [7:0] = compl_status -*/ - ctrl->mcc_tag_status[tag] = CQE_VALID_MASK; - ctrl->mcc_tag_status[tag] |= (wrb_idx << CQE_STATUS_WRB_SHIFT); - ctrl->mcc_tag_status[tag] |= (extd_status << CQE_STATUS_ADDL_SHIFT) & -CQE_STATUS_ADDL_MASK; - ctrl->mcc_tag_status[tag] |= (compl_status & CQE_STATUS_MASK); - - /* write ordering implied in wake_up_interruptible */ - clear_bit(MCC_TAG_STATE_RUNNING, &ctrl->ptag_state[tag].tag_state); - wake_up_interruptible(&ctrl->mcc_wait[tag]); - return 0; -} - -static struct be_mcc_compl *be_mcc_compl_get(struct beiscsi_hba *phba) -{ - struct be_queue_info *mcc_cq = &phba->ctrl.mcc_obj.cq; - struct be_mcc_compl *compl = queue_tail_node(mcc_cq); - - if (be_mcc_compl_is_new(compl)) { - queue_tail_inc(mcc_cq); - return compl; - } - return NULL; -} - /** * beiscsi_fail_session(): Closing session with appropriate error * @cls_session: ptr to session @@ -528,27 +458,65 @@ void beiscsi_process_async_event(struct beiscsi_hba *phba, evt_code, compl->status, compl->flags); } -int beiscsi_process_mcc(struct beiscsi_hba *phba) +int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, + struct be_mcc_compl *compl) { - struct be_mcc_compl *compl; - int num = 0, status = 0; - struct be_ctrl_info *ctrl = &phba->ctrl; + struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); + u16 compl_status, extd_status; + struct be_dma_mem *tag_mem; + unsigned int tag, wrb_idx; - while ((compl = be_mcc_compl_get(phba))) { - if (compl->flags & CQE_FLAGS_ASYNC_MASK) { - beiscsi_process_async_event(phba, compl); - } else if (compl->flags & CQE_FLAGS_COMPLETED_MASK) { - status = be_mcc_compl_process(ctrl, compl); - atomic_dec(&phba->ctrl.mcc_obj.q.used); - } - be_mcc_compl_use(compl); - num++; + /** +* Just swap the status to host endian; mcc tag is opaquely co
[PATCH 02/12] be2iscsi: Use macros for MCC WRB and CQE fields
Rename mcc_numtag to mcc_tag_status. MCC CQE status is processed using macros already defined in be_cmds.h. Add MCC_Q_WRB_ and MCC_Q_CMD_TAG_MASK macros to map to already defined CQE_STATUS_ macros to be consistent when posting MCC. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be.h | 8 +++- drivers/scsi/be2iscsi/be_cmds.c | 40 +--- drivers/scsi/be2iscsi/be_cmds.h | 13 +++-- drivers/scsi/be2iscsi/be_main.c | 11 ++- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h index 1524fe4..da1d87a 100644 --- a/drivers/scsi/be2iscsi/be.h +++ b/drivers/scsi/be2iscsi/be.h @@ -135,7 +135,7 @@ struct be_ctrl_info { wait_queue_head_t mcc_wait[MAX_MCC_CMD + 1]; unsigned int mcc_tag[MAX_MCC_CMD]; - unsigned int mcc_numtag[MAX_MCC_CMD + 1]; + unsigned int mcc_tag_status[MAX_MCC_CMD + 1]; unsigned short mcc_alloc_index; unsigned short mcc_free_index; unsigned int mcc_tag_available; @@ -145,6 +145,12 @@ struct be_ctrl_info { #include "be_cmds.h" +/* WRB index mask for MCC_Q_LEN queue entries */ +#define MCC_Q_WRB_IDX_MASK CQE_STATUS_WRB_MASK +#define MCC_Q_WRB_IDX_SHIFTCQE_STATUS_WRB_SHIFT +/* TAG is from 1...MAX_MCC_CMD, MASK includes MAX_MCC_CMD */ +#define MCC_Q_CMD_TAG_MASK ((MAX_MCC_CMD << 1) - 1) + #define PAGE_SHIFT_4K 12 #define PAGE_SIZE_4K (1 << PAGE_SHIFT_4K) #define mcc_timeout12 /* 12s timeout */ diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index e8e9d22..c5e7739 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -125,7 +125,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) if (phba->ctrl.mcc_tag_available) { tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index]; phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0; - phba->ctrl.mcc_numtag[tag] = 0; + phba->ctrl.mcc_tag_status[tag] = 0; phba->ctrl.ptag_state[tag].tag_state = 0; } if (tag) { @@ -157,7 +157,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, struct be_dma_mem *mbx_cmd_mem) { int rc = 0; - uint32_t mcc_tag_response; + uint32_t mcc_tag_status; uint16_t status = 0, addl_status = 0, wrb_num = 0; struct be_mcc_wrb *temp_wrb; struct be_cmd_req_hdr *mbx_hdr; @@ -172,7 +172,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, /* wait for the mccq completion */ rc = wait_event_interruptible_timeout( phba->ctrl.mcc_wait[tag], - phba->ctrl.mcc_numtag[tag], + phba->ctrl.mcc_tag_status[tag], msecs_to_jiffies( BEISCSI_HOST_MBX_TIMEOUT)); /** @@ -209,15 +209,15 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, } rc = 0; - mcc_tag_response = phba->ctrl.mcc_numtag[tag]; - status = (mcc_tag_response & CQE_STATUS_MASK); - addl_status = ((mcc_tag_response & CQE_STATUS_ADDL_MASK) >> + mcc_tag_status = phba->ctrl.mcc_tag_status[tag]; + status = (mcc_tag_status & CQE_STATUS_MASK); + addl_status = ((mcc_tag_status & CQE_STATUS_ADDL_MASK) >> CQE_STATUS_ADDL_SHIFT); if (mbx_cmd_mem) { mbx_hdr = (struct be_cmd_req_hdr *)mbx_cmd_mem->va; } else { - wrb_num = (mcc_tag_response & CQE_STATUS_WRB_MASK) >> + wrb_num = (mcc_tag_status & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; temp_wrb = (struct be_mcc_wrb *)queue_get_wrb(mccq, wrb_num); mbx_hdr = embedded_payload(temp_wrb); @@ -257,7 +257,7 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba, void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) { spin_lock(&ctrl->mcc_lock); - tag = tag & 0x00FF; + tag = tag & MCC_Q_CMD_TAG_MASK; ctrl->mcc_tag[ctrl->mcc_free_index] = tag; if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) ctrl->mcc_free_index = 0; @@ -334,10 +334,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl, struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); u16 compl_status, extd_status; struct be_dma_mem *tag_mem; - unsigned short tag; + unsigned int tag, wrb_idx; be_dws_le_to_cpu(compl, 4); - tag = (compl->tag0 & 0x00FF); + tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); + wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; if (!test_bit(MCC_TAG_STATE_RUNNING, &ctrl->ptag_state[tag].tag_state)) { @@ -366,17 +367,18 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl, }
[PATCH 07/12] be2iscsi: Cleanup processing of BMBX completion
Remove confusingly named be_mcc_compl_is_new and be_mcc_compl_use functions in processing of BMBX. Rearrange beiscsi_process_mbox_compl function. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_cmds.c | 75 - 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index 60db2de..728aa133 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -263,21 +263,6 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, return rc; } -static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl) -{ - if (compl->flags != 0) { - compl->flags = le32_to_cpu(compl->flags); - WARN_ON((compl->flags & CQE_FLAGS_VALID_MASK) == 0); - return true; - } else - return false; -} - -static inline void be_mcc_compl_use(struct be_mcc_compl *compl) -{ - compl->flags = 0; -} - /* * beiscsi_process_mbox_compl()- Check the MBX completion status * @ctrl: Function specific MBX data structure @@ -298,30 +283,46 @@ static int beiscsi_process_mbox_compl(struct be_ctrl_info *ctrl, struct be_cmd_req_hdr *hdr = embedded_payload(wrb); struct be_cmd_resp_hdr *resp_hdr; - be_dws_le_to_cpu(compl, 4); + /** +* To check if valid bit is set, check the entire word as we don't know +* the endianness of the data (old entry is host endian while a new +* entry is little endian) +*/ + if (!compl->flags) { + beiscsi_log(phba, KERN_ERR, + BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, + "BC_%d : BMBX busy, no completion\n"); + return -EBUSY; + } + compl->flags = le32_to_cpu(compl->flags); + WARN_ON((compl->flags & CQE_FLAGS_VALID_MASK) == 0); + /** +* Just swap the status to host endian; +* mcc tag is opaquely copied from mcc_wrb. +*/ + be_dws_le_to_cpu(compl, 4); compl_status = (compl->status >> CQE_STATUS_COMPL_SHIFT) & - CQE_STATUS_COMPL_MASK; - if (compl_status != MCC_STATUS_SUCCESS) { - extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & - CQE_STATUS_EXTD_MASK; + CQE_STATUS_COMPL_MASK; + extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) & + CQE_STATUS_EXTD_MASK; + /* Need to reset the entire word that houses the valid bit */ + compl->flags = 0; - beiscsi_log(phba, KERN_ERR, - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, - "BC_%d : error in cmd completion: " - "Subsystem : %d Opcode : %d " - "status(compl/extd)=%d/%d\n", - hdr->subsystem, hdr->opcode, - compl_status, extd_status); - - if (compl_status == MCC_STATUS_INSUFFICIENT_BUFFER) { - resp_hdr = (struct be_cmd_resp_hdr *) hdr; - if (resp_hdr->response_length) - return 0; - } - return -EINVAL; + if (compl_status == MCC_STATUS_SUCCESS) + return 0; + + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, + "BC_%d : error in cmd completion: Subsystem : %d Opcode : %d status(compl/extd)=%d/%d\n", + hdr->subsystem, hdr->opcode, compl_status, extd_status); + + if (compl_status == MCC_STATUS_INSUFFICIENT_BUFFER) { + /* if status is insufficient buffer, check the length */ + resp_hdr = (struct be_cmd_resp_hdr *) hdr; + if (resp_hdr->response_length) + return 0; } - return 0; + return -EINVAL; } static void beiscsi_process_async_link(struct beiscsi_hba *phba, @@ -453,10 +454,6 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, struct be_dma_mem *tag_mem; unsigned int tag, wrb_idx; - /** -* Just swap the status to host endian; mcc tag is opaquely copied -* from mcc_wrb -*/ be_dws_le_to_cpu(compl, 4); tag = (compl->tag0 & MCC_Q_CMD_TAG_MASK); wrb_idx = (compl->tag0 & CQE_STATUS_WRB_MASK) >> CQE_STATUS_WRB_SHIFT; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/12] be2iscsi: _bh for io_sgl_lock and mgmt_sgl_lock
Processing of mgmt and IO tasks are done in process context and sofitrqs. Allocation and freeing of sgl_handles needs to be done under spin_lock_bh/spin_unlock_bh and move the locks to the routines. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_main.c | 25 ++--- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 03265b6..fa2b589 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -1132,6 +1132,7 @@ static struct sgl_handle *alloc_io_sgl_handle(struct beiscsi_hba *phba) { struct sgl_handle *psgl_handle; + spin_lock_bh(&phba->io_sgl_lock); if (phba->io_sgl_hndl_avbl) { beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO, "BM_%d : In alloc_io_sgl_handle," @@ -1149,12 +1150,14 @@ static struct sgl_handle *alloc_io_sgl_handle(struct beiscsi_hba *phba) phba->io_sgl_alloc_index++; } else psgl_handle = NULL; + spin_unlock_bh(&phba->io_sgl_lock); return psgl_handle; } static void free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) { + spin_lock_bh(&phba->io_sgl_lock); beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_IO, "BM_%d : In free_,io_sgl_free_index=%d\n", phba->io_sgl_free_index); @@ -1169,6 +1172,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) "value there=%p\n", phba->io_sgl_free_index, phba->io_sgl_hndl_base [phba->io_sgl_free_index]); +spin_unlock_bh(&phba->io_sgl_lock); return; } phba->io_sgl_hndl_base[phba->io_sgl_free_index] = psgl_handle; @@ -1177,6 +1181,7 @@ free_io_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) phba->io_sgl_free_index = 0; else phba->io_sgl_free_index++; + spin_unlock_bh(&phba->io_sgl_lock); } static inline struct wrb_handle * @@ -1257,6 +1262,7 @@ static struct sgl_handle *alloc_mgmt_sgl_handle(struct beiscsi_hba *phba) { struct sgl_handle *psgl_handle; + spin_lock_bh(&phba->mgmt_sgl_lock); if (phba->eh_sgl_hndl_avbl) { psgl_handle = phba->eh_sgl_hndl_base[phba->eh_sgl_alloc_index]; phba->eh_sgl_hndl_base[phba->eh_sgl_alloc_index] = NULL; @@ -1274,13 +1280,14 @@ static struct sgl_handle *alloc_mgmt_sgl_handle(struct beiscsi_hba *phba) phba->eh_sgl_alloc_index++; } else psgl_handle = NULL; + spin_unlock_bh(&phba->mgmt_sgl_lock); return psgl_handle; } void free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) { - + spin_lock_bh(&phba->mgmt_sgl_lock); beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, "BM_%d : In free_mgmt_sgl_handle," "eh_sgl_free_index=%d\n", @@ -1295,6 +1302,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) "BM_%d : Double Free in eh SGL ," "eh_sgl_free_index=%d\n", phba->eh_sgl_free_index); + spin_unlock_bh(&phba->mgmt_sgl_lock); return; } phba->eh_sgl_hndl_base[phba->eh_sgl_free_index] = psgl_handle; @@ -1304,6 +1312,7 @@ free_mgmt_sgl_handle(struct beiscsi_hba *phba, struct sgl_handle *psgl_handle) phba->eh_sgl_free_index = 0; else phba->eh_sgl_free_index++; + spin_unlock_bh(&phba->mgmt_sgl_lock); } static void @@ -4616,11 +4625,9 @@ beiscsi_free_mgmt_task_handles(struct beiscsi_conn *beiscsi_conn, } if (io_task->psgl_handle) { - spin_lock_bh(&phba->mgmt_sgl_lock); free_mgmt_sgl_handle(phba, io_task->psgl_handle); io_task->psgl_handle = NULL; - spin_unlock_bh(&phba->mgmt_sgl_lock); } if (io_task->mtask_addr) { @@ -4666,9 +4673,7 @@ static void beiscsi_cleanup_task(struct iscsi_task *task) } if (io_task->psgl_handle) { - spin_lock(&phba->io_sgl_lock); free_io_sgl_handle(phba, io_task->psgl_handle); - spin_unlock(&phba->io_sgl_lock); io_task->psgl_handle = NULL; } @@ -4784,9 +4789,7 @@ static int beiscsi_alloc_pdu(struct iscsi_task *task, uint8_t opcode) io_task->pwrb_handle = NULL; if (task->sc) { - spin_lock(&phba->io_sgl_lock); io_task->psgl_handle = alloc_io_sgl_handle(phba); - spin_unlock(&phba->io_sgl_lock);
[PATCH 06/12] be2iscsi: Fix be_mcc_compl_poll to use tag_state
be_mcc_compl_poll waits till 'used' count of MCC WRBQ is zero. This is to determine the completion of an MCC sent. Change function to poll for the tag of MCC sent, instead, and wait till its tag_state is cleared. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_cmds.c | 92 + 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index 12b60dd..60db2de 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -104,19 +104,6 @@ int be_chk_reset_complete(struct beiscsi_hba *phba) return 0; } -void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag) -{ - struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; - u32 val = 0; - - set_bit(MCC_TAG_STATE_RUNNING, &phba->ctrl.ptag_state[tag].tag_state); - val |= mccq->id & DB_MCCQ_RING_ID_MASK; - val |= 1 << DB_MCCQ_NUM_POSTED_SHIFT; - /* ring doorbell after all of request and state is written */ - wmb(); - iowrite32(val, phba->db_va + DB_MCCQ_OFFSET); -} - unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) { unsigned int tag = 0; @@ -139,6 +126,28 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) return tag; } +void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) +{ + spin_lock_bh(&ctrl->mcc_lock); + tag = tag & MCC_Q_CMD_TAG_MASK; + ctrl->mcc_tag[ctrl->mcc_free_index] = tag; + if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) + ctrl->mcc_free_index = 0; + else + ctrl->mcc_free_index++; + ctrl->mcc_tag_available++; + spin_unlock_bh(&ctrl->mcc_lock); +} + +/** + * beiscsi_fail_session(): Closing session with appropriate error + * @cls_session: ptr to session + **/ +void beiscsi_fail_session(struct iscsi_cls_session *cls_session) +{ + iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED); +} + /* * beiscsi_mccq_compl_wait()- Process completion in MCC CQ * @phba: Driver private structure @@ -254,19 +263,6 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, return rc; } -void free_mcc_tag(struct be_ctrl_info *ctrl, unsigned int tag) -{ - spin_lock(&ctrl->mcc_lock); - tag = tag & MCC_Q_CMD_TAG_MASK; - ctrl->mcc_tag[ctrl->mcc_free_index] = tag; - if (ctrl->mcc_free_index == (MAX_MCC_CMD - 1)) - ctrl->mcc_free_index = 0; - else - ctrl->mcc_free_index++; - ctrl->mcc_tag_available++; - spin_unlock(&ctrl->mcc_lock); -} - static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl) { if (compl->flags != 0) { @@ -328,15 +324,6 @@ static int beiscsi_process_mbox_compl(struct be_ctrl_info *ctrl, return 0; } -/** - * beiscsi_fail_session(): Closing session with appropriate error - * @cls_session: ptr to session - **/ -void beiscsi_fail_session(struct iscsi_cls_session *cls_session) -{ - iscsi_session_failure(cls_session->dd_data, ISCSI_ERR_CONN_FAILED); -} - static void beiscsi_process_async_link(struct beiscsi_hba *phba, struct be_mcc_compl *compl) { @@ -532,6 +519,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, **/ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned int tag) { + struct be_ctrl_info *ctrl = &phba->ctrl; int i; for (i = 0; i < mcc_timeout; i++) { @@ -540,19 +528,33 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned int tag) beiscsi_process_mcc_cq(phba); - if (atomic_read(&phba->ctrl.mcc_obj.q.used) == 0) + if (!test_bit(MCC_TAG_STATE_RUNNING, + &ctrl->ptag_state[tag].tag_state)) break; udelay(100); } - if (i == mcc_timeout) { - beiscsi_log(phba, KERN_ERR, - BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, - "BC_%d : FW Timed Out\n"); - phba->fw_timeout = true; - beiscsi_ue_detect(phba); - return -EBUSY; - } - return 0; + + if (i < mcc_timeout) + return 0; + + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, + "BC_%d : FW Timed Out\n"); + phba->fw_timeout = true; + beiscsi_ue_detect(phba); + return -EBUSY; +} + +void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag) +{ + struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q; + u32 val = 0; + + set_bit(MCC_TAG_STATE_RUNNING, &phba->ctrl.ptag_state[tag].tag_state); + val |= mccq->id & DB_MCCQ_RING_ID_MASK; + val |= 1 << DB_MCCQ_NUM_POSTED_SHIFT; + /* make request available for DMA */ + wmb(); + iowrite32(val, phba->db_va + DB_MCCQ_OFFSET); } /* -- 2.5.0 -- To unsubscribe from this list:
[PATCH 10/12] be2iscsi: Fix ExpStatSn in management tasks
Connection resets observed from some targets when NOP-Out with wrong ExpStatSn is sent. FW keeps track of StatSn and fills up ExpStatSn accordingly. The header filled up by the stack needs to be modified by driver to clear ExpStatSn. If the field is not cleared, FW recalculates ExpStatSn and wrong offset'ed ExpStatSn is seen in the wire trace. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_main.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 3f08a11..03265b6 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -4926,7 +4926,6 @@ int beiscsi_iotask_v2(struct iscsi_task *task, struct scatterlist *sg, pwrb = io_task->pwrb_handle->pwrb; - io_task->cmd_bhs->iscsi_hdr.exp_statsn = 0; io_task->bhs_len = sizeof(struct be_cmd_bhs); if (writedir) { @@ -4987,7 +4986,6 @@ static int beiscsi_iotask(struct iscsi_task *task, struct scatterlist *sg, unsigned int doorbell = 0; pwrb = io_task->pwrb_handle->pwrb; - io_task->cmd_bhs->iscsi_hdr.exp_statsn = 0; io_task->bhs_len = sizeof(struct be_cmd_bhs); if (writedir) { @@ -5159,23 +5157,21 @@ static int beiscsi_task_xmit(struct iscsi_task *task) { struct beiscsi_io_task *io_task = task->dd_data; struct scsi_cmnd *sc = task->sc; - struct beiscsi_hba *phba = NULL; + struct beiscsi_hba *phba; struct scatterlist *sg; int num_sg; unsigned int writedir = 0, xferlen = 0; - phba = ((struct beiscsi_conn *)task->conn->dd_data)->phba; + if (!io_task->conn->login_in_progress) + task->hdr->exp_statsn = 0; if (!sc) return beiscsi_mtask(task); io_task->scsi_cmnd = sc; num_sg = scsi_dma_map(sc); + phba = io_task->conn->phba; if (num_sg < 0) { - struct iscsi_conn *conn = task->conn; - struct beiscsi_hba *phba = NULL; - - phba = ((struct beiscsi_conn *)conn->dd_data)->phba; beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_IO | BEISCSI_LOG_ISCSI, "BM_%d : scsi_dma_map Failed " -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/12] be2iscsi: Remove unused mcc_cq_lock
mcc_cq_lock spin_lock is used only in beiscsi_process_mcc which is called only when all interrupts are disabled from mgmt_epfw_cleanup during unloading of driver. There is no other context where there can be contention for the processing of CQ. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be.h | 1 - drivers/scsi/be2iscsi/be_cmds.c | 2 -- drivers/scsi/be2iscsi/be_main.c | 1 - 3 files changed, 4 deletions(-) diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h index 7d425af..1524fe4 100644 --- a/drivers/scsi/be2iscsi/be.h +++ b/drivers/scsi/be2iscsi/be.h @@ -132,7 +132,6 @@ struct be_ctrl_info { /* MCC Rings */ struct be_mcc_obj mcc_obj; spinlock_t mcc_lock;/* For serializing mcc cmds to BE card */ - spinlock_t mcc_cq_lock; wait_queue_head_t mcc_wait[MAX_MCC_CMD + 1]; unsigned int mcc_tag[MAX_MCC_CMD]; diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index 34c33d4..e8e9d22 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -532,7 +532,6 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba) int num = 0, status = 0; struct be_ctrl_info *ctrl = &phba->ctrl; - spin_lock_bh(&phba->ctrl.mcc_cq_lock); while ((compl = be_mcc_compl_get(phba))) { if (compl->flags & CQE_FLAGS_ASYNC_MASK) { beiscsi_process_async_event(phba, compl); @@ -547,7 +546,6 @@ int beiscsi_process_mcc(struct beiscsi_hba *phba) if (num) hwi_ring_cq_db(phba, phba->ctrl.mcc_obj.cq.id, num, 1); - spin_unlock_bh(&phba->ctrl.mcc_cq_lock); return status; } diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 70179e1..314fd2c 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -730,7 +730,6 @@ static int be_ctrl_init(struct beiscsi_hba *phba, struct pci_dev *pdev) memset(mbox_mem_align->va, 0, sizeof(struct be_mcc_mailbox)); mutex_init(&ctrl->mbox_lock); spin_lock_init(&phba->ctrl.mcc_lock); - spin_lock_init(&phba->ctrl.mcc_cq_lock); return status; } -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/12] be2iscsi: Fix MCC WRB leak in open_connection
In open with IP of unknown address family, only tag is freed and error returned. MCC WRB allocated for the operation is not freed. Added check for supported family of IP in the beginning before allocating the tag and WRB. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_mgmt.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c index 85044b8..ccac1d7 100644 --- a/drivers/scsi/be2iscsi/be_mgmt.c +++ b/drivers/scsi/be2iscsi/be_mgmt.c @@ -829,6 +829,13 @@ int mgmt_open_connection(struct beiscsi_hba *phba, unsigned short cid = beiscsi_ep->ep_cid; struct be_sge *sge; + if (dst_addr->sa_family != PF_INET && dst_addr->sa_family != PF_INET6) { + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, + "BG_%d : unknown addr family %d\n", + dst_addr->sa_family); + return -EINVAL; + } + phwi_ctrlr = phba->phwi_ctrlr; phwi_context = phwi_ctrlr->phwi_ctxt; @@ -868,7 +875,8 @@ int mgmt_open_connection(struct beiscsi_hba *phba, beiscsi_ep->dst_addr = daddr_in->sin_addr.s_addr; beiscsi_ep->dst_tcpport = ntohs(daddr_in->sin_port); beiscsi_ep->ip_type = BE2_IPV4; - } else if (dst_addr->sa_family == PF_INET6) { + } else { + /* else its PF_INET6 family */ req->ip_address.ip_type = BE2_IPV6; memcpy(&req->ip_address.addr, &daddr_in6->sin6_addr.in6_u.u6_addr8, 16); @@ -877,14 +885,6 @@ int mgmt_open_connection(struct beiscsi_hba *phba, memcpy(&beiscsi_ep->dst6_addr, &daddr_in6->sin6_addr.in6_u.u6_addr8, 16); beiscsi_ep->ip_type = BE2_IPV6; - } else{ - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, - "BG_%d : unknown addr family %d\n", - dst_addr->sa_family); - mutex_unlock(&ctrl->mbox_lock); - free_mcc_tag(&phba->ctrl, tag); - return -EINVAL; - } req->cid = cid; i = phba->nxt_cqid++; -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/12] be2iscsi: Rename MCC and BMBX processing functions
beiscsi_mccq_compl -> beiscsi_mccq_compl_wait - indicate blocking call. be_mcc_wait_compl -> be_mcc_compl_poll - indicate polling for completion. be_mbox_db_ready_wait -> be_mbox_db_ready_poll - indicate polling for RDY. be_mcc_compl_process -> beiscsi_process_mbox_compl - indicate BMBX compl. Signed-off-by: Jitendra Bhivare --- drivers/scsi/be2iscsi/be_cmds.c | 35 +-- drivers/scsi/be2iscsi/be_cmds.h | 6 +++--- drivers/scsi/be2iscsi/be_iscsi.c | 8 drivers/scsi/be2iscsi/be_main.c | 8 drivers/scsi/be2iscsi/be_mgmt.c | 12 ++-- 5 files changed, 34 insertions(+), 35 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c index fa010ac..8dd8521 100644 --- a/drivers/scsi/be2iscsi/be_cmds.c +++ b/drivers/scsi/be2iscsi/be_cmds.c @@ -140,7 +140,7 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) } /* - * beiscsi_mccq_compl()- Wait for completion of MBX + * beiscsi_mccq_compl_wait()- Process completion in MCC CQ * @phba: Driver private structure * @tag: Tag for the MBX Command * @wrb: the WRB used for the MBX Command @@ -152,9 +152,9 @@ unsigned int alloc_mcc_tag(struct beiscsi_hba *phba) * Success: 0 * Failure: Non-Zero **/ -int beiscsi_mccq_compl(struct beiscsi_hba *phba, - uint32_t tag, struct be_mcc_wrb **wrb, - struct be_dma_mem *mbx_cmd_mem) +int beiscsi_mccq_compl_wait(struct beiscsi_hba *phba, + uint32_t tag, struct be_mcc_wrb **wrb, + struct be_dma_mem *mbx_cmd_mem) { int rc = 0; uint32_t mcc_tag_status; @@ -283,7 +283,7 @@ static inline void be_mcc_compl_use(struct be_mcc_compl *compl) } /* - * be_mcc_compl_process()- Check the MBX comapletion status + * beiscsi_process_mbox_compl()- Check the MBX completion status * @ctrl: Function specific MBX data structure * @compl: Completion status of MBX Command * @@ -293,8 +293,8 @@ static inline void be_mcc_compl_use(struct be_mcc_compl *compl) * Success: Zero * Failure: Non-Zero **/ -static int be_mcc_compl_process(struct be_ctrl_info *ctrl, - struct be_mcc_compl *compl) +static int beiscsi_process_mbox_compl(struct be_ctrl_info *ctrl, + struct be_mcc_compl *compl) { u16 compl_status, extd_status; struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem); @@ -520,7 +520,7 @@ int beiscsi_process_mcc_compl(struct be_ctrl_info *ctrl, } /* - * be_mcc_wait_compl()- Wait for MBX completion + * be_mcc_compl_poll()- Wait for MBX completion * @phba: driver private structure * * Wait till no more pending mcc requests are present @@ -556,8 +556,7 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned int tag) } /* -/* - * be_mbox_db_ready_wait()- Check ready status + * be_mbox_db_ready_poll()- Check ready status * @ctrl: Function specific MBX data structure * * Check for the ready status of FW to send BMBX @@ -567,7 +566,7 @@ int be_mcc_compl_poll(struct beiscsi_hba *phba, unsigned int tag) * Success: 0 * Failure: Non-Zero **/ -static int be_mbox_db_ready_wait(struct be_ctrl_info *ctrl) +static int be_mbox_db_ready_poll(struct be_ctrl_info *ctrl) { /* wait 30s for generic non-flash MBOX operation */ #define BEISCSI_MBX_RDY_BIT_TIMEOUT3 @@ -629,7 +628,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) struct be_mcc_compl *compl = &mbox->compl; struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev); - status = be_mbox_db_ready_wait(ctrl); + status = be_mbox_db_ready_poll(ctrl); if (status) return status; @@ -638,7 +637,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) val |= (upper_32_bits(mbox_mem->dma) >> 2) << 2; iowrite32(val, db); - status = be_mbox_db_ready_wait(ctrl); + status = be_mbox_db_ready_poll(ctrl); if (status) return status; @@ -648,7 +647,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) val |= (u32) (mbox_mem->dma >> 4) << 2; iowrite32(val, db); - status = be_mbox_db_ready_wait(ctrl); + status = be_mbox_db_ready_poll(ctrl); if (status) return status; @@ -656,7 +655,7 @@ int be_mbox_notify(struct be_ctrl_info *ctrl) udelay(1); if (be_mcc_compl_is_new(compl)) { - status = be_mcc_compl_process(ctrl, &mbox->compl); + status = beiscsi_process_mbox_compl(ctrl, compl); be_mcc_compl_use(compl); if (status) { beiscsi_log(phba, KERN_ERR, @@ -689,7 +688,7 @@ static int be_mbox_notify_wait(struct beiscsi_hba *phba) struct be_mcc_compl *compl = &mbox->compl; struct be_ctrl_info *ctrl = &phba->ctrl; - status = be_mbox_db_ready_wait(ctrl); + status = be_mbox_db_ready_poll(ctrl); if (status)
Re: [PATCH 5/7] scsi: qla4xxx: shut up warning for rd_reg_indirect
On 27/01/16 9:27 PM, "Arnd Bergmann" wrote: >The qla4_83xx_rd_reg_indirect() function can fail when it is unable >to read a register, but not all callers check its return value before >using the register data, and gcc correctly warns about this: > >qla4xxx/ql4_83xx.c: In function 'qla4_83xx_process_reset_template': >qla4xxx/ql4_83xx.c:1073:36: warning: 'value' may be used uninitialized in >this function > ha->reset_tmplt.array[index++] = value; >^ >qla4xxx/ql4_83xx.c:1050:11: note: 'value' was declared here > uint32_t value; > ^ >qla4xxx/ql4_83xx.c:902:8: warning: 'value' may be used uninitialized in >this function > value &= p_rmw_hdr->test_mask; >^ >qla4xxx/ql4_83xx.c:895:11: note: 'value' was declared here > uint32_t value; > ^ >In file included from ../include/linux/io.h:25:0, > from ../include/linux/pci.h:31, > from ../drivers/scsi/qla4xxx/ql4_def.h:16, > from ../drivers/scsi/qla4xxx/ql4_83xx.c:10: >asm/io.h:101:2: warning: 'value' may be used uninitialized in this >function > asm volatile("str %1, %0" > ^ >qla4xxx/ql4_83xx.c:874:11: note: 'value' was declared here > uint32_t value; > ^ > >Unfortunately, I don't see any helpful way to add proper error handling >for this case, and the failure scenario for rd_reg seems rather obscure, >so this bails out and makes the rd_reg accessor set the result to >0x >so we at least get a predictable value. > >Signed-off-by: Arnd Bergmann >--- > drivers/scsi/qla4xxx/ql4_83xx.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > >diff --git a/drivers/scsi/qla4xxx/ql4_83xx.c >b/drivers/scsi/qla4xxx/ql4_83xx.c >index 5d4f8e67fb25..638f72c5ab05 100644 >--- a/drivers/scsi/qla4xxx/ql4_83xx.c >+++ b/drivers/scsi/qla4xxx/ql4_83xx.c >@@ -46,11 +46,13 @@ int qla4_83xx_rd_reg_indirect(struct scsi_qla_host >*ha, uint32_t addr, > > ret_val = qla4_83xx_set_win_base(ha, addr); > >- if (ret_val == QLA_SUCCESS) >+ if (ret_val == QLA_SUCCESS) { > *data = qla4_83xx_rd_reg(ha, QLA83XX_WILDCARD); >- else >+ } else { >+ *data = 0x; > ql4_printk(KERN_ERR, ha, "%s: failed read of addr 0x%x!\n", > __func__, addr); >+ } > > return ret_val; > } >-- >2.7.0 > Acked-by: Nilesh Javali -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html