[GIT PULL] SCSI fixes for 4.3-rc4
This is three essential bug fixes for various SCSI parts. The only affected users are SCSI multi-path via device handler (basically all the enterprise) and mvsas users. The dh bugs are an async entanglement in boot resulting in a serious WARN_ON trip and a use after free on remove leading to a crash with strict memory accounting. The mvsas bug manifests as a null deref oops but only on abort sequences; however, these can commonly occur with SATA attached devices, hence the fix. The patch is available here: git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes The short changelog is: Christoph Hellwig (1): scsi_dh: don't try to load a device handler during async probing Dāvis Mosāns (1): mvsas: Fix NULL pointer dereference in mvs_slot_task_free Junichi Nomura (1): scsi_dh: fix use-after-free when removing scsi device And the diffstat: drivers/scsi/mvsas/mv_sas.c | 2 ++ drivers/scsi/scsi_dh.c | 8 ++-- drivers/scsi/scsi_priv.h| 2 ++ drivers/scsi/scsi_sysfs.c | 2 ++ 4 files changed, 12 insertions(+), 2 deletions(-) With full diff below. James --- diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c index 454536c..9c78074 100644 --- a/drivers/scsi/mvsas/mv_sas.c +++ b/drivers/scsi/mvsas/mv_sas.c @@ -887,6 +887,8 @@ static void mvs_slot_free(struct mvs_info *mvi, u32 rx_desc) static void mvs_slot_task_free(struct mvs_info *mvi, struct sas_task *task, struct mvs_slot_info *slot, u32 slot_idx) { + if (!slot) + return; if (!slot->task) return; if (!sas_protocol_ata(task->task_proto)) diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c index 0a2168e..e7649ed 100644 --- a/drivers/scsi/scsi_dh.c +++ b/drivers/scsi/scsi_dh.c @@ -226,16 +226,20 @@ int scsi_dh_add_device(struct scsi_device *sdev) drv = scsi_dh_find_driver(sdev); if (drv) - devinfo = scsi_dh_lookup(drv); + devinfo = __scsi_dh_lookup(drv); if (devinfo) err = scsi_dh_handler_attach(sdev, devinfo); return err; } -void scsi_dh_remove_device(struct scsi_device *sdev) +void scsi_dh_release_device(struct scsi_device *sdev) { if (sdev->handler) scsi_dh_handler_detach(sdev); +} + +void scsi_dh_remove_device(struct scsi_device *sdev) +{ device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr); } diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h index 644bb73..4d01cdb 100644 --- a/drivers/scsi/scsi_priv.h +++ b/drivers/scsi/scsi_priv.h @@ -173,9 +173,11 @@ extern struct async_domain scsi_sd_probe_domain; /* scsi_dh.c */ #ifdef CONFIG_SCSI_DH int scsi_dh_add_device(struct scsi_device *sdev); +void scsi_dh_release_device(struct scsi_device *sdev); void scsi_dh_remove_device(struct scsi_device *sdev); #else static inline int scsi_dh_add_device(struct scsi_device *sdev) { return 0; } +static inline void scsi_dh_release_device(struct scsi_device *sdev) { } static inline void scsi_dh_remove_device(struct scsi_device *sdev) { } #endif diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index b89..dff8faf 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,6 +399,8 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) sdev = container_of(work, struct scsi_device, ew.work); + scsi_dh_release_device(sdev); + parent = sdev->sdev_gendev.parent; spin_lock_irqsave(sdev->host->host_lock, flags); -- 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] sg: Fix double-free when drives detach during SG_IO
In sg_common_write(), we free the block request and return -ENODEV if the device is detached in the middle of the SG_IO ioctl(). Unfortunately, sg_finish_rem_req() also tries to free srp->rq, so we end up freeing rq->cmd in the already free rq object, and then free the object itself out from under the current user. This ends up corrupting random memory via the list_head on the rq object. The most common crash trace I saw is this: [ cut here ] kernel BUG at block/blk-core.c:1420! Call Trace: [] blk_put_request+0x5b/0x80 [] sg_finish_rem_req+0x6b/0x120 [sg] [] sg_common_write.isra.14+0x459/0x5a0 [sg] [] ? selinux_file_alloc_security+0x48/0x70 [] sg_new_write.isra.17+0x195/0x2d0 [sg] [] sg_ioctl+0x644/0xdb0 [sg] [] do_vfs_ioctl+0x90/0x520 [] ? file_has_perm+0x97/0xb0 [] SyS_ioctl+0x91/0xb0 [] tracesys+0xdd/0xe2 RIP [] __blk_put_request+0x154/0x1a0 The solution is straightforward: just set srp->rq to NULL in the failure branch so that sg_finish_rem_req() doesn't attempt to re-free it. Additionally, since sg_rq_end_io() will never be called on the object when this happens, we need to free memory backing ->cmd if it isn't embedded in the object itself. KASAN was extremely helpful in finding the root cause of this bug. Signed-off-by: Calvin Owens --- drivers/scsi/sg.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 9d7b7db..503ab8b 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -787,8 +787,14 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp, return k; /* probably out of space --> ENOMEM */ } if (atomic_read(&sdp->detaching)) { - if (srp->bio) + if (srp->bio) { + if (srp->rq->cmd != srp->rq->__cmd) + kfree(srp->rq->cmd); + blk_end_request_all(srp->rq, -EIO); + srp->rq = NULL; + } + sg_finish_rem_req(srp); return -ENODEV; } -- 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
Re: [Y2038] [RESEND PATCH v3] scsi: stex: Remove use of struct timeval
On Friday 30 October 2015 01:54:10 Tina Ruchandani wrote: > > > > Thanks for the conversion. Can you please check if other (scsi) drivers > > have the same y2038 issues? A quick "git grep do_gettimeofday > > drivers/scsi/ | wc -l" reveals 30 occurrences (of cause not all are > > problematic). In fact all of them are problematic, just for different reasons. * Some drivers actually overflow in 2038 in a way that causes problems in those drivers. These obviously need to be fixed right away. * A second class of drivers pass time_t/timeval/timespec values to or from user space. Even in cases where the absolute numbers are small (monotonic times, or time intervals), we have to change them to be able to deal with 32-bit user space that will be compiled against a modified libc using 64-bit time_t. * All other driver are likely not broken, but we want to change them anyway, to annotate the fact that we have looked at them. My goal is to remove the definition of time_t (and all derived structures) from the kernel once all drivers have been converted, to ensure that we are not adding new broken users, and to have a reasonable confidence that we have in actually found the ones that were wrong. > Hi Johannes, > Yes, there are quite a few occurrences of timeval within scsi. I had > sent some of the trivial back in the Feb-May 2015 period, and they > were ack-ed by my then mentor and a couple of other people, but not > merged or ack-ed by someone from linux-scsi. Until today, I thought > using "RESEND" would be impolite, but now I will resend the other ones > as well. Arnd Bergmann is leading this effort and may have more > insightful comments. > I provided a "Reviewed-by" tag in https://lkml.org/lkml/2015/5/5/201 . Normally, when patches get picked up directly from the list, the person who merges it should add the tags directly. However, if you have to re-send the patch (with or without small modifications), you should add that tag after your Signed-off-by, so it does not get lost when the new patch is applied. Arnd -- 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 24/25] hpsa: add in sas transport class
> On Oct 30, 2015, at 5:00 PM, Don Brace wrote: > On 10/30/2015 03:07 PM, Matthew R. Ochs wrote: >>> On Oct 28, 2015, at 5:06 PM, Don Brace wrote: >>> >>> From: Kevin Barnett >>> >>> Reviewed-by: Scott Teel >>> Reviewed-by: Justin Lindley >>> Reviewed-by: Kevin Barnett >>> Signed-off-by: Don Brace >>> --- >>> drivers/scsi/hpsa.c | 535 >>> +-- >>> drivers/scsi/hpsa.h | 27 ++ >>> drivers/scsi/hpsa_cmd.h | 14 + >>> 3 files changed, 555 insertions(+), 21 deletions(-) >>> >>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c >>> index 56526312..ca38a00 100644 >>> --- a/drivers/scsi/hpsa.c >>> +++ b/drivers/scsi/hpsa.c >>> @@ -41,6 +41,7 @@ >>> >>> +static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h, >>> + unsigned char *scsi3addr) >>> +{ >>> + struct ReportExtendedLUNdata *physdev; >>> + u32 nphysicals; >>> + u64 sa = 0; >>> + int i; >>> + >>> + physdev = kzalloc(sizeof(*physdev), GFP_KERNEL); >>> + if (!physdev) >>> + return 0; >>> + >>> + if (hpsa_scsi_do_report_phys_luns(h, physdev, sizeof(*physdev))) { >>> + dev_err(&h->pdev->dev, "report physical LUNs failed.\n"); >>> + kfree(physdev); >>> + return 0; >>> + } >>> + nphysicals = get_unaligned_be32(physdev->LUNListLength) / 24; >>> + >>> + for (i = 0; i < nphysicals; i++) >>> + if (!memcmp(&physdev->LUN[i].lunid[0], scsi3addr, 8)) >>> + sa = get_unaligned_be64(&physdev->LUN[i].wwid[0]); >> Don't you want to break out here if you found a match? > Agreed. >> >>> + >>> + kfree(physdev); >>> + >>> + return sa; >>> +} >>> + >>> +static void hpsa_get_sas_address(struct ctlr_info *h, unsigned char >>> *scsi3addr, >>> + struct hpsa_scsi_dev_t *dev) >>> +{ >>> + int rc; >>> + u64 sa = 0; >>> + >>> + if (is_hba_lunid(scsi3addr)) { >>> + struct bmic_sense_subsystem_info *ssi; >>> + >>> + ssi = kzalloc(sizeof(*ssi), GFP_KERNEL); >> What happens if this allocation fails? If the I/O can succeed without the >> DMA buffer then you will run into trouble when deriving sa. > Added check for NULL. With these changes Reviewed-by: Matthew R. Ochs -- 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_sas : Add locking to megasas_aen_polling
Apologies for missing that - it has been a while since submitting to the stable tree. I'll resubmit the same patch with the Cc: sta...@vger.kernel.org in the sign-off area. Regards, Ben On Fri, Oct 30, 2015 at 5:20 PM, Greg KH wrote: > On Fri, Oct 30, 2015 at 01:47:50PM -0400, Ben Guthro wrote: >> From: Glenn Watkins >> >> Under conditions of offlining drives, and rescanning the scsi host, >> we can get into situations that the megasas_aen_polling kthread >> can crash(GPF) in the megasas_aen_polling work queue: >> >> [ 1206.568641] general protection fault: [#1] SMP >> [ 1206.569479] Modules linked in: xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 >> xt_state nf_conntrack iptable_filter ip_tables x_tables coretemp >> crct10dif_pclmul crc32_pclmul aesni_intel ablk_helper cryptd psmouse lrw >> vmwgfx gf128mul serio_raw glue_helper aes_x86_64 ppdev ttm microcode >> vmw_balloon drm_kms_helper drm parport_pc parport fb_sys_fops sysimgblt >> sysfillrect syscopyarea vmw_vmci binfmt_misc floppy mptspi mptscsih >> vmw_pvscsi megaraid_sas pata_acpi mptbase vmxnet3 >> [ 1206.576488] CPU: 0 PID: 1157 Comm: kworker/0:2 Not tainted 4.3.0-rc7-svt1 >> #1 >> [ 1206.577520] Hardware name: VMware, Inc. VMware Virtual Platform/440BX >> Desktop Reference Platform, BIOS 6.00 04/14/2014 >> [ 1206.579101] Workqueue: events megasas_aen_polling [megaraid_sas] >> [ 1206.580007] task: 8818bb7b8000 ti: 8818ca28 task.ti: >> 8818ca28 >> [ 1206.581104] RIP: 0010:[] [] >> bdi_unregister+0x3d/0x1e0 >> [ 1206.582339] RSP: 0018:8818ca283cb8 EFLAGS: 00010246 >> [ 1206.583131] RAX: dead0200 RBX: 8818bb603f08 RCX: >> 8818c6487800 >> [ 1206.584184] RDX: 8818bb603f08 RSI: 7fff RDI: >> 81f9aa68 >> [ 1206.585243] RBP: 8818ca283d18 R08: R09: >> >> [ 1206.586294] R10: 000fffe0 R11: dead0200 R12: >> 8818bb6042f0 >> [ 1206.587346] R13: 8818bb604530 R14: 00ae R15: >> 0080 >> [ 1206.588388] FS: () GS:88193fc0() >> knlGS: >> [ 1206.589598] CS: 0010 DS: ES: CR0: 8005003b >> [ 1206.590457] CR2: 01a89000 CR3: 0018c07f2000 CR4: >> 000406f0 >> [ 1206.591545] Stack: >> [ 1206.591870] 8818bb6042f0 8818bb603d78 00ae >> 0080 >> [ 1206.593098] 8818ca283ce8 8108f683 8818ca283d18 >> 813332b0 >> [ 1206.594308] 8818ca283d18 8818bb603d78 8818bb6042f0 >> 8818bb604530 >> [ 1206.595532] Call Trace: >> [ 1206.595922] [] ? cancel_delayed_work_sync+0x13/0x20 >> [ 1206.596903] [] ? blk_sync_queue+0x80/0x90 >> [ 1206.597753] [] blk_cleanup_queue+0x114/0x150 >> [ 1206.598645] [] __scsi_remove_device+0x54/0xd0 >> [ 1206.599556] [] scsi_remove_device+0x2f/0x50 >> [ 1206.600441] [] megasas_aen_polling+0x34d/0x670 >> [megaraid_sas] >> [ 1206.601561] [] process_one_work+0x14c/0x400 >> [ 1206.602449] [] worker_thread+0x117/0x480 >> [ 1206.603295] [] ? create_worker+0x1c0/0x1c0 >> [ 1206.604160] [] kthread+0xc9/0xe0 >> [ 1206.604898] [] ? flush_kthread_worker+0x90/0x90 >> [ 1206.605831] [] ret_from_fork+0x3f/0x70 >> [ 1206.606659] [] ? flush_kthread_worker+0x90/0x90 >> [ 1206.607585] Code: c7 c7 68 aa f9 81 48 83 ec 48 e8 bf 76 59 00 48 8b 43 >> 08 48 8b 13 49 bb 00 02 00 00 00 00 ad de 48 c7 c7 68 aa f9 81 48 89 42 08 >> <48> 89 10 4c 89 5b 08 e8 27 76 59 00 e8 32 92 f4 ff 48 8d 7b 50 >> [ 1206.611938] RIP [] bdi_unregister+0x3d/0x1e0 >> [ 1206.612856] RSP >> >> This can be readily reproduced by a pair of shell scripts - one of which >> loops on >> onlining / offlining drives via MegaCli (or storcli, if you prefer) >> >> #!/bin/bash >> >> while [ 1 ]; do >> /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:0] a0 &>2 >> /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:11] a0 &>2 >> >> /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:0] a0 &>2 >> /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:11] a0 &>2 >> done >> >> Meanwhile, the second script is looping on rescanning the scsi hosts: >> >> #!/bin/bash >> while [ 1 ]; do >> for (( l=0; l<4; l++ )); do >> echo - - - > /sys/class/scsi_host/host$l/scan >> done >> done >> >> This was originally introduced in the following commit: >> >> commit 7e8a75f4dfbff173977b2f58799c3eceb7b09afd >> Author: Yang, Bo >> Date: Tue Oct 6 14:50:17 2009 -0600 >> >> [SCSI] megaraid_sas: Add the support for updating the OS after >> adding/removing the devices from FW >> >> The fix for this is to add some locking around the AEN polling. >> Since this affects all kernels since 2.6.33, I have also CC'ed the stable >> list. >> >> Signed-off-by: Glenn Watkins >> Signed-off-by: Ben Guthro >> --- >> drivers/scsi/megaraid/megaraid_sas_base.c |2 ++ >> 1 file changed, 2 insertions(+) >> > > > > Thi
[PATCH 2/2] Restart list search after unlock in scsi_remove_target
When dropping a lock while iterating a list we must restart the search as other threads could have manipulated the list under us. Without this we can get stuck in an endless loop. This is a slightly modified version of a patch from Christoph Hellwig (see also https://www.spinics.net/lists/linux-scsi/msg89416.html). Reported-by: Johannes Thumshirn Signed-off-by: Bart Van Assche Cc: Johannes Thumshirn Cc: Christoph Hellwig Cc: Dan Williams Cc: stable --- drivers/scsi/scsi_sysfs.c | 16 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index b9fb61a..5a183d1 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1158,32 +1158,24 @@ static void __scsi_remove_target(struct scsi_target *starget) void scsi_remove_target(struct device *dev) { struct Scsi_Host *shost = dev_to_shost(dev->parent); - struct scsi_target *starget, *last = NULL; + struct scsi_target *starget; unsigned long flags; - /* remove targets being careful to lookup next entry before -* deleting the last -*/ +restart: spin_lock_irqsave(shost->host_lock, flags); list_for_each_entry(starget, &shost->__targets, siblings) { if (starget->reaped) continue; if (starget->dev.parent == dev || &starget->dev == dev) { - /* assuming new targets arrive at the end */ kref_get(&starget->reap_ref); starget->reaped = true; spin_unlock_irqrestore(shost->host_lock, flags); - if (last) - scsi_target_reap(last); - last = starget; __scsi_remove_target(starget); - spin_lock_irqsave(shost->host_lock, flags); + scsi_target_reap(starget); + goto restart; } } spin_unlock_irqrestore(shost->host_lock, flags); - - if (last) - scsi_target_reap(last); } EXPORT_SYMBOL(scsi_remove_target); -- 2.6.2.307.g37023ba -- 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 3/5] ipr: Inquiry IOA page 0xC4 during initialization.
On 10/30/2015 11:49 AM, Gabriel Krisman Bertazi wrote: > diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h > index 69257c4..f0238cc 100644 > --- a/drivers/scsi/ipr.h > +++ b/drivers/scsi/ipr.h > @@ -849,6 +849,16 @@ struct ipr_inquiry_page0 { > u8 page[IPR_INQUIRY_PAGE0_ENTRIES]; > }__attribute__((packed)); > > +struct ipr_inquiry_pageC4 { > + u8 peri_qual_dev_type; > + u8 page_code; > + u8 reserved1; > + u8 len; > + u8 cache_cap[4]; > +#define IPR_CAP_SYNC_CACHE 0x08 > + u8 reserved2[20]; > +}__attribute__((packed)); This triggers a couple complaints from checkpatch. You need a space after the }, which checkpatch flags as an error and __packed is preferred to __attribute__((packed)), which is flagged as a warning. There are a couple of other warnings that checkpatch issues in this series, so you might want to take a look at them, fixup, and resend. We should probably spin a follow up patch to fix up the __attribute__ usage in ipr.h, since you were simply being consistent with that. Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- 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 1/2] Separate target visibility from reaped state information
Instead of representing the states "visible in sysfs" and "has been removed from the target list" by a single state variable, use two variables to represent this information. Signed-off-by: Bart Van Assche Cc: Johannes Thumshirn Cc: Christoph Hellwig Cc: Dan Williams Cc: stable --- drivers/scsi/scsi_scan.c | 31 +++ drivers/scsi/scsi_sysfs.c | 7 --- include/scsi/scsi_device.h | 3 ++- 3 files changed, 9 insertions(+), 32 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index f9f3f82..80b8c3f 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -314,7 +314,6 @@ static void scsi_target_destroy(struct scsi_target *starget) struct Scsi_Host *shost = dev_to_shost(dev->parent); unsigned long flags; - starget->state = STARGET_DEL; transport_destroy_device(dev); spin_lock_irqsave(shost->host_lock, flags); if (shost->hostt->target_destroy) @@ -379,19 +378,15 @@ static void scsi_target_reap_ref_release(struct kref *kref) struct scsi_target *starget = container_of(kref, struct scsi_target, reap_ref); - /* -* if we get here and the target is still in the CREATED state that -* means it was allocated but never made visible (because a scan -* turned up no LUNs), so don't call device_del() on it. -*/ - if (starget->state != STARGET_CREATED) { + if (starget->visible) { + starget->visible = false; transport_remove_device(&starget->dev); device_del(&starget->dev); } scsi_target_destroy(starget); } -static void scsi_target_reap_ref_put(struct scsi_target *starget) +void scsi_target_reap(struct scsi_target *starget) { kref_put(&starget->reap_ref, scsi_target_reap_ref_release); } @@ -437,7 +432,6 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, starget->can_queue = 0; INIT_LIST_HEAD(&starget->siblings); INIT_LIST_HEAD(&starget->devices); - starget->state = STARGET_CREATED; starget->scsi_level = SCSI_2; starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED; retry: @@ -498,25 +492,6 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, } /** - * scsi_target_reap - check to see if target is in use and destroy if not - * @starget: target to be checked - * - * This is used after removing a LUN or doing a last put of the target - * it checks atomically that nothing is using the target and removes - * it if so. - */ -void scsi_target_reap(struct scsi_target *starget) -{ - /* -* serious problem if this triggers: STARGET_DEL is only set in the if -* the reap_ref drops to zero, so we're trying to do another final put -* on an already released kref -*/ - BUG_ON(starget->state == STARGET_DEL); - scsi_target_reap_ref_put(starget); -} - -/** * sanitize_inquiry_string - remove non-graphical chars from an INQUIRY result string * @s: INQUIRY result string to sanitize * @len: length of the string diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index b89..b9fb61a 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -974,7 +974,7 @@ static int scsi_target_add(struct scsi_target *starget) { int error; - if (starget->state != STARGET_CREATED) + if (starget->visible) return 0; error = device_add(&starget->dev); @@ -983,7 +983,7 @@ static int scsi_target_add(struct scsi_target *starget) return error; } transport_add_device(&starget->dev); - starget->state = STARGET_RUNNING; + starget->visible = true; pm_runtime_set_active(&starget->dev); pm_runtime_enable(&starget->dev); @@ -1166,11 +1166,12 @@ void scsi_remove_target(struct device *dev) */ spin_lock_irqsave(shost->host_lock, flags); list_for_each_entry(starget, &shost->__targets, siblings) { - if (starget->state == STARGET_DEL) + if (starget->reaped) continue; if (starget->dev.parent == dev || &starget->dev == dev) { /* assuming new targets arrive at the end */ kref_get(&starget->reap_ref); + starget->reaped = true; spin_unlock_irqrestore(shost->host_lock, flags); if (last) scsi_target_reap(last); diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index fe89d7c..a734656 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -267,6 +267,8 @@ struct scsi_target { unsigned intexpecting_lun_change:1; /* A device has reported * a 3F/0E UA, other devices on
[PATCH 0/2] Fix a hard lockup in scsi_remove_target()
On October 2, 2015 Johannes Thumshirn reported the following hard lockup (see also http://thread.gmane.org/gmane.linux.kernel/2052359): Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 0 [...] Call Trace: [] dump_trace+0x7d/0x2d0 [] show_stack_log_lvl+0x94/0x170 [] show_stack+0x21/0x50 [] dump_stack+0x41/0x51 [] panic+0xc8/0x1d7 [] watchdog_overflow_callback+0xba/0xc0 [] __perf_event_overflow+0x88/0x240 [] intel_pmu_handle_irq+0x1fa/0x3e0 [] perf_event_nmi_handler+0x26/0x40 [] nmi_handle.isra.2+0x8d/0x180 [] do_nmi+0x126/0x3c0 [] end_repeat_nmi+0x1a/0x1e [] scsi_remove_target+0x68/0x240 [scsi_mod] [] process_one_work+0x172/0x420 [] worker_thread+0x11a/0x3c0 [] kthread+0xb4/0xc0 [] ret_from_fork+0x58/0x90 The following two patches fix this lockup: 0001-Separate-target-visibility-from-reaped-state-informa.patch 0002-Restart-list-search-after-unlock-in-scsi_remove_targ.patch These patches have been tested with the SRP initiator (ib_srp). -- 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 24/25] hpsa: add in sas transport class
On 10/30/2015 03:07 PM, Matthew R. Ochs wrote: On Oct 28, 2015, at 5:06 PM, Don Brace wrote: From: Kevin Barnett Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 535 +-- drivers/scsi/hpsa.h | 27 ++ drivers/scsi/hpsa_cmd.h | 14 + 3 files changed, 555 insertions(+), 21 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 56526312..ca38a00 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -41,6 +41,7 @@ +static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h, + unsigned char *scsi3addr) +{ + struct ReportExtendedLUNdata *physdev; + u32 nphysicals; + u64 sa = 0; + int i; + + physdev = kzalloc(sizeof(*physdev), GFP_KERNEL); + if (!physdev) + return 0; + + if (hpsa_scsi_do_report_phys_luns(h, physdev, sizeof(*physdev))) { + dev_err(&h->pdev->dev, "report physical LUNs failed.\n"); + kfree(physdev); + return 0; + } + nphysicals = get_unaligned_be32(physdev->LUNListLength) / 24; + + for (i = 0; i < nphysicals; i++) + if (!memcmp(&physdev->LUN[i].lunid[0], scsi3addr, 8)) + sa = get_unaligned_be64(&physdev->LUN[i].wwid[0]); Don't you want to break out here if you found a match? Agreed. + + kfree(physdev); + + return sa; +} + +static void hpsa_get_sas_address(struct ctlr_info *h, unsigned char *scsi3addr, + struct hpsa_scsi_dev_t *dev) +{ + int rc; + u64 sa = 0; + + if (is_hba_lunid(scsi3addr)) { + struct bmic_sense_subsystem_info *ssi; + + ssi = kzalloc(sizeof(*ssi), GFP_KERNEL); What happens if this allocation fails? If the I/O can succeed without the DMA buffer then you will run into trouble when deriving sa. Added check for NULL. -- 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] megaraid_sas : Add locking to megasas_aen_polling
On Fri, Oct 30, 2015 at 01:47:50PM -0400, Ben Guthro wrote: > From: Glenn Watkins > > Under conditions of offlining drives, and rescanning the scsi host, > we can get into situations that the megasas_aen_polling kthread > can crash(GPF) in the megasas_aen_polling work queue: > > [ 1206.568641] general protection fault: [#1] SMP > [ 1206.569479] Modules linked in: xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 > xt_state nf_conntrack iptable_filter ip_tables x_tables coretemp > crct10dif_pclmul crc32_pclmul aesni_intel ablk_helper cryptd psmouse lrw > vmwgfx gf128mul serio_raw glue_helper aes_x86_64 ppdev ttm microcode > vmw_balloon drm_kms_helper drm parport_pc parport fb_sys_fops sysimgblt > sysfillrect syscopyarea vmw_vmci binfmt_misc floppy mptspi mptscsih > vmw_pvscsi megaraid_sas pata_acpi mptbase vmxnet3 > [ 1206.576488] CPU: 0 PID: 1157 Comm: kworker/0:2 Not tainted 4.3.0-rc7-svt1 > #1 > [ 1206.577520] Hardware name: VMware, Inc. VMware Virtual Platform/440BX > Desktop Reference Platform, BIOS 6.00 04/14/2014 > [ 1206.579101] Workqueue: events megasas_aen_polling [megaraid_sas] > [ 1206.580007] task: 8818bb7b8000 ti: 8818ca28 task.ti: > 8818ca28 > [ 1206.581104] RIP: 0010:[] [] > bdi_unregister+0x3d/0x1e0 > [ 1206.582339] RSP: 0018:8818ca283cb8 EFLAGS: 00010246 > [ 1206.583131] RAX: dead0200 RBX: 8818bb603f08 RCX: > 8818c6487800 > [ 1206.584184] RDX: 8818bb603f08 RSI: 7fff RDI: > 81f9aa68 > [ 1206.585243] RBP: 8818ca283d18 R08: R09: > > [ 1206.586294] R10: 000fffe0 R11: dead0200 R12: > 8818bb6042f0 > [ 1206.587346] R13: 8818bb604530 R14: 00ae R15: > 0080 > [ 1206.588388] FS: () GS:88193fc0() > knlGS: > [ 1206.589598] CS: 0010 DS: ES: CR0: 8005003b > [ 1206.590457] CR2: 01a89000 CR3: 0018c07f2000 CR4: > 000406f0 > [ 1206.591545] Stack: > [ 1206.591870] 8818bb6042f0 8818bb603d78 00ae > 0080 > [ 1206.593098] 8818ca283ce8 8108f683 8818ca283d18 > 813332b0 > [ 1206.594308] 8818ca283d18 8818bb603d78 8818bb6042f0 > 8818bb604530 > [ 1206.595532] Call Trace: > [ 1206.595922] [] ? cancel_delayed_work_sync+0x13/0x20 > [ 1206.596903] [] ? blk_sync_queue+0x80/0x90 > [ 1206.597753] [] blk_cleanup_queue+0x114/0x150 > [ 1206.598645] [] __scsi_remove_device+0x54/0xd0 > [ 1206.599556] [] scsi_remove_device+0x2f/0x50 > [ 1206.600441] [] megasas_aen_polling+0x34d/0x670 > [megaraid_sas] > [ 1206.601561] [] process_one_work+0x14c/0x400 > [ 1206.602449] [] worker_thread+0x117/0x480 > [ 1206.603295] [] ? create_worker+0x1c0/0x1c0 > [ 1206.604160] [] kthread+0xc9/0xe0 > [ 1206.604898] [] ? flush_kthread_worker+0x90/0x90 > [ 1206.605831] [] ret_from_fork+0x3f/0x70 > [ 1206.606659] [] ? flush_kthread_worker+0x90/0x90 > [ 1206.607585] Code: c7 c7 68 aa f9 81 48 83 ec 48 e8 bf 76 59 00 48 8b 43 08 > 48 8b 13 49 bb 00 02 00 00 00 00 ad de 48 c7 c7 68 aa f9 81 48 89 42 08 <48> > 89 10 4c 89 5b 08 e8 27 76 59 00 e8 32 92 f4 ff 48 8d 7b 50 > [ 1206.611938] RIP [] bdi_unregister+0x3d/0x1e0 > [ 1206.612856] RSP > > This can be readily reproduced by a pair of shell scripts - one of which > loops on > onlining / offlining drives via MegaCli (or storcli, if you prefer) > > #!/bin/bash > > while [ 1 ]; do > /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:0] a0 &>2 > /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:11] a0 &>2 > > /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:0] a0 &>2 > /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:11] a0 &>2 > done > > Meanwhile, the second script is looping on rescanning the scsi hosts: > > #!/bin/bash > while [ 1 ]; do > for (( l=0; l<4; l++ )); do > echo - - - > /sys/class/scsi_host/host$l/scan > done > done > > This was originally introduced in the following commit: > > commit 7e8a75f4dfbff173977b2f58799c3eceb7b09afd > Author: Yang, Bo > Date: Tue Oct 6 14:50:17 2009 -0600 > > [SCSI] megaraid_sas: Add the support for updating the OS after > adding/removing the devices from FW > > The fix for this is to add some locking around the AEN polling. > Since this affects all kernels since 2.6.33, I have also CC'ed the stable > list. > > Signed-off-by: Glenn Watkins > Signed-off-by: Ben Guthro > --- > drivers/scsi/megaraid/megaraid_sas_base.c |2 ++ > 1 file 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/maj
Re: [PATCH 1 21/25] hpsa: disable report lun data caching
On 10/30/2015 09:25 AM, Tomas Henzl wrote: On 28.10.2015 23:06, Don Brace wrote: From: Scott Teel When external target arrays are present, disable the firmware's normal behavior of returning a cached copy of the report lun data, and force it to collect new data each time we request a report luns. This is necessary for external arrays, since there may be no reliable signal from the external array to the smart array when lun configuration changes, and thus when driver requests report luns, it may be stale data. Use diag options to turn off RPL data caching. Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 86 +++ drivers/scsi/hpsa_cmd.h |3 ++ 2 files changed, 89 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index e521acd..33fd0aa 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, static void hpsa_command_resubmit_worker(struct work_struct *work); static u32 lockup_detected(struct ctlr_info *h); static int detect_controller_lockup(struct ctlr_info *h); +static void hpsa_disable_rld_caching(struct ctlr_info *h); static int hpsa_luns_changed(struct ctlr_info *h); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) @@ -6386,6 +6387,24 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, c->Request.CDB[8] = (size >> 8) & 0xFF; c->Request.CDB[9] = size & 0xFF; break; + case BMIC_SENSE_DIAG_OPTIONS: + c->Request.CDBLen = 16; + c->Request.type_attr_dir = + TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ); + c->Request.Timeout = 0; + /* Spec says this should be BMIC_WRITE */ + c->Request.CDB[0] = BMIC_READ; + c->Request.CDB[6] = BMIC_SENSE_DIAG_OPTIONS; + break; + case BMIC_SET_DIAG_OPTIONS: + c->Request.CDBLen = 16; + c->Request.type_attr_dir = + TYPE_ATTR_DIR(cmd_type, + ATTR_SIMPLE, XFER_WRITE); + c->Request.Timeout = 0; + c->Request.CDB[0] = BMIC_WRITE; + c->Request.CDB[6] = BMIC_SET_DIAG_OPTIONS; + break; case HPSA_CACHE_FLUSH: c->Request.CDBLen = 12; c->Request.type_attr_dir = @@ -8086,6 +8105,7 @@ static void hpsa_rescan_ctlr_worker(struct work_struct *work) hpsa_scan_start(h->scsi_host); scsi_host_put(h->scsi_host); } else if (h->discovery_polling) { + hpsa_disable_rld_caching(h); if (hpsa_luns_changed(h)) { struct Scsi_Host *sh = NULL; @@ -8423,6 +8443,72 @@ out: kfree(flush_buf); } +/* Make controller gather fresh report lun data each time we + * send down a report luns request + */ +static void hpsa_disable_rld_caching(struct ctlr_info *h) +{ + u32 *options; + struct CommandList *c; + int rc; + + /* Don't bother trying to set diag options if locked up */ + if (unlikely(h->lockup_detected)) + return; + + options = kzalloc(sizeof(*options), GFP_KERNEL); + if (!options) { + dev_err(&h->pdev->dev, + "Error: failed to disable rld caching, during alloc.\n"); + return; + } + + c = cmd_alloc(h); + + /* first, get the current diag options settings */ + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, + RAID_CTLR_LUNID, TYPE_CMD)) + goto errout; + + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, + PCI_DMA_FROMDEVICE, NO_TIMEOUT); + if ((rc != 0) || (c->err_info->CommandStatus != 0)) + goto errout; + + /* Now, set the bit for disabling the RLD caching */ + *options |= HPSA_DIAG_OPTS_DISABLE_RLD_CACHING; + + if (fill_cmd(c, BMIC_SET_DIAG_OPTIONS, h, options, 4, 0, + RAID_CTLR_LUNID, TYPE_CMD)) + goto errout; + + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, + PCI_DMA_TODEVICE, NO_TIMEOUT); + if ((rc != 0) || (c->err_info->CommandStatus != 0)) + goto errout; + + /* Now verify that it got set: */ + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, + RAID_CTLR_LUNID, TYPE_CMD)) + goto errout; + + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, + PCI_DMA_FROMDEVICE, NO_TIMEOUT); +
Re: [PATCH 1 15/25] hpsa: enhance hpsa_get_device_id
On 10/30/2015 03:08 AM, Hannes Reinecke wrote: On 10/28/2015 11:06 PM, Don Brace wrote: use an index into vpd data for SAS/SATA drives Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 23 --- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c1b053f..1361414 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3137,7 +3137,7 @@ out: /* Get the device id from inquiry page 0x83 */ static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr, - unsigned char *device_id, int buflen) + unsigned char *device_id, int index, int buflen) { int rc; unsigned char *buf; @@ -3149,8 +3149,10 @@ static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr, return -ENOMEM; rc = hpsa_scsi_do_inquiry(h, scsi3addr, VPD_PAGE | 0x83, buf, 64); if (rc == 0) - memcpy(device_id, &buf[8], buflen); + memcpy(device_id, &buf[index], buflen); + kfree(buf); + return rc != 0; } @@ -3379,6 +3381,18 @@ static int hpsa_device_supports_aborts(struct ctlr_info *h, return rc; } +static void sanitize_inquiry_string(unsigned char *s, int len) +{ + bool terminated = false; + + for (; len > 0; (--len, ++s)) { + if (*s == 0) + terminated = true; + if (terminated || *s < 0x20 || *s > 0x7e) + *s = ' '; + } +} + static int hpsa_update_device_info(struct ctlr_info *h, unsigned char scsi3addr[], struct hpsa_scsi_dev_t *this_device, unsigned char *is_OBDR_device) I would prefer to have the function from scsi_scan.c to be exported. Hardly a point to duplicate it. Cheers, Hannes I can submit a patch to change it. Can it be in a later patch? -- 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.3-rc7] be2iscsi : Fix bogus WARN_ON length check
Hi Tim, tim.gard...@canonical.com writes: > From: Tim Gardner > > drivers/scsi/be2iscsi/be_main.c: In function 'be_sgl_create_contiguous': > drivers/scsi/be2iscsi/be_main.c:3187:18: warning: logical not is only applied > to the left hand side of comparison [-Wlogical-not-parentheses] > WARN_ON(!length > 0); > > gcc version 5.2.1 This patch (or similar) was already posted on Oct 1 by Joel Stanley. See http://comments.gmane.org/gmane.linux.scsi/105462 Thanks, Johannes -- 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 08/25] hpsa: fix hpsa_adjust_hpsa_scsi_table
On 10/30/2015 02:57 AM, Hannes Reinecke wrote: On 10/28/2015 11:05 PM, Don Brace wrote: Fix a NULL pointer issue in the driver when devices are removed during a reset. Signed-off-by: Don Brace --- drivers/block/cciss.h |1 + drivers/scsi/hpsa.c | 16 drivers/scsi/hpsa.h |1 + 3 files changed, 18 insertions(+) diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h index 7fda30e..f8b8c6b 100644 --- a/drivers/block/cciss.h +++ b/drivers/block/cciss.h @@ -155,6 +155,7 @@ struct ctlr_info size_t reply_pool_size; unsigned char reply_pool_wraparound; u32 *blockFetchTable; + u8 reset_in_progress; }; /* Defining the diffent access_methods Why do you need to add it here? To my knowledge cciss and hpsa are different drivers ... ctag issue. corrected. Cheers, Hannes -- 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 06/25] hpsa: abandon rescans on memory alloaction failures.
On 10/30/2015 02:53 AM, Hannes Reinecke wrote: On 10/28/2015 11:05 PM, Don Brace wrote: Abandon and reschedule rescan process only if device inquiries fail due to mem alloc failures, which are likely to occur for all devices. Otherwise, skip device if inquiry fails for other reasons, and continue rescanning process for other devices. Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Reviewed-by: Kevin Barnett Reviewed-by: Tomas Henzl Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a3f671c..ecc6ada 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3375,10 +3375,13 @@ static int hpsa_update_device_info(struct ctlr_info *h, unsigned char *inq_buff; unsigned char *obdr_sig; + int rc = 0; inq_buff = kzalloc(OBDR_TAPE_INQ_SIZE, GFP_KERNEL); - if (!inq_buff) + if (!inq_buff) { + rc = -ENOMEM; goto bail_out; + } /* Do an inquiry to the device to see what it is. */ if (hpsa_scsi_do_inquiry(h, scsi3addr, 0, inq_buff, @@ -3386,6 +3389,7 @@ static int hpsa_update_device_info(struct ctlr_info *h, /* Inquiry failed (msg printed already) */ dev_err(&h->pdev->dev, "hpsa_update_device_info: inquiry failed\n"); + rc = 1; goto bail_out; } Why '1' and not a normal error code, seeing that you're using -ENOMEM above? Changed to -EIO @@ -3435,7 +3439,7 @@ static int hpsa_update_device_info(struct ctlr_info *h, bail_out: kfree(inq_buff); - return 1; + return rc; } static void hpsa_update_device_supports_aborts(struct ctlr_info *h, @@ -3803,6 +3807,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) n_ext_target_devs = 0; for (i = 0; i < nphysicals + nlogicals + 1; i++) { u8 *lunaddrbytes, is_OBDR = 0; + int rc = 0; /* Figure out where the LUN ID info is coming from */ lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position, @@ -3815,11 +3820,20 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) continue; /* Get device type, vendor, model, device id */ - if (hpsa_update_device_info(h, lunaddrbytes, tmpdevice, - &is_OBDR)) { + rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice, + &is_OBDR); + if (rc == -ENOMEM) { + dev_warn(&h->pdev->dev, + "Out of memory, rescan deferred.\n"); h->drv_req_rescan = 1; - continue; /* skip it if we can't talk to it. */ + goto out; } + if (rc) { + dev_warn(&h->pdev->dev, + "Inquiry failed, skipping device.\n"); + continue; + } + figure_bus_target_lun(h, lunaddrbytes, tmpdevice); hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes); this_device = currentsd[ncurrent]; Cheers, Hannes -- 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: Blk-mq/scsi-mq Tuning
On 10/30/2015 10:00 AM, Hannes Reinecke wrote: > On 10/30/2015 03:12 PM, Chad Dupuis wrote: >> >> >> On Fri, 30 Oct 2015, Hannes Reinecke wrote: >> >>> On 10/30/2015 02:25 PM, Chad Dupuis wrote: On Fri, 30 Oct 2015, Hannes Reinecke wrote: > On 10/28/2015 09:11 PM, Chad Dupuis wrote: >> Hi Folks, >> >> We¹ve begun to explore blk-mq and scsi-mq and wanted to know if there >> were >> any best practices in terms of block layer settings. We¹re looking >> specifically at the FCoE and iSCSI protocols. >> >> A little background on the queues in our hardware first: we have a per >> connection transmit queue and multiple, global receive queues. The >> transmit queues are not pegged to a particular CPU. The receive >> queues >> are pegged to the first N CPUs where N is the number of receive >> queues. >> We set the nr_hw_queues in the scsi_host_template to N as well. >> > Weelll ... I think you'll run into issues here. > The whole point of the multiqueue implementation is that you can tag > the > submission _and_ completion queue to a single CPU, thereby eliminating > locking. > If you only peg the completion queue to a CPU you'll still have > contention on the submission queue, needing to take locks etc. > > Plus you will _inevitably_ incur cache misses, as the completion will > basically never occur on the same CPU which did the submissoin. > Hence the context needs to be bounced to the CPU holding the completion > queue, or you'll need to do a IPI to inform the submitting CPU. > But if you do that you're essentially doing single-queue submission, > so I doubt we're seeing that great improvements. This was why I was asking if there was a blk-mq API to be able to set CPU affinity for the hardware context queues so I could steer the submissions to the CPUs that my receive queues are on (even if they are allowed to float). >>> But what would that achieve? >>> Each of the hardware context queues would still having to use the >>> same submission queue, so you'd have to have some serialisation >>> with spinlocks et.al. during submission. Which is what blk-mq >>> tries to avoid. >>> Am I wrong? >> >> Sadly, no I believe you're correct. So essentially the upshot seems to >> be if you can have a 1x1 request:response queue then sticking with the >> older queuecommand method is better? >> > Hmm; you might be getting some performance improvements as the > submission path from the blocklayer down is more efficient, but in > your case the positive effects might be eliminated by reducing the > number of receive queues. > But then you never know until you try :-) > > The alternative would indeed be to move to MC/S with block-mq; that > should give you some benefits as you'd be able to utilize several queues. > I have actually discussed that with Emulex; moving to MC/S in the iSCSI > stack might indeed be viable when using blk-mq. It would be a rather > good match with the existing blk-mq implementation, and most of the > implementation would be in the iSCSI stack, reducing the burden on the > driver vendors :-) > I think the mulit session mq stuff would actually just work too. It was done with hw iscsi in mind. MC/s might be nicer in their case though. For qla4xxx type of cards, would all the MC/S stuff be done in firmware, so all you need is a common interface to expose the connection details and then some common code to map them to hw queues? -- 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 25/25] hpsa: bump the driver version
Reviewed-by: Matthew R. Ochs -- 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 24/25] hpsa: add in sas transport class
> On Oct 28, 2015, at 5:06 PM, Don Brace wrote: > > From: Kevin Barnett > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 535 +-- > drivers/scsi/hpsa.h | 27 ++ > drivers/scsi/hpsa_cmd.h | 14 + > 3 files changed, 555 insertions(+), 21 deletions(-) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 56526312..ca38a00 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -41,6 +41,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -205,6 +206,16 @@ static struct board_type products[] = { > {0x103C, "Unknown Smart Array", &SA5_access}, > }; > > +static struct scsi_transport_template *hpsa_sas_transport_template; > +static int hpsa_add_sas_host(struct ctlr_info *h); > +static void hpsa_delete_sas_host(struct ctlr_info *h); > +static int hpsa_add_sas_device(struct hpsa_sas_node *hpsa_sas_node, > + struct hpsa_scsi_dev_t *device); > +static void hpsa_remove_sas_device(struct hpsa_scsi_dev_t *device); > +static struct hpsa_scsi_dev_t > + *hpsa_find_device_by_sas_rphy(struct ctlr_info *h, > + struct sas_rphy *rphy); > + > #define SCSI_CMD_BUSY ((struct scsi_cmnd *)&hpsa_cmd_busy) > static const struct scsi_cmnd hpsa_cmd_busy; > #define SCSI_CMD_IDLE ((struct scsi_cmnd *)&hpsa_cmd_idle) > @@ -277,6 +288,8 @@ static void hpsa_command_resubmit_worker(struct > work_struct *work); > static u32 lockup_detected(struct ctlr_info *h); > static int detect_controller_lockup(struct ctlr_info *h); > static void hpsa_disable_rld_caching(struct ctlr_info *h); > +static inline int hpsa_scsi_do_report_phys_luns(struct ctlr_info *h, > + struct ReportExtendedLUNdata *buf, int bufsize); > static int hpsa_luns_changed(struct ctlr_info *h); > > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > @@ -1696,8 +1709,12 @@ static int hpsa_add_device(struct ctlr_info *h, struct > hpsa_scsi_dev_t *device) > { > int rc = 0; > > - rc = scsi_add_device(h->scsi_host, device->bus, > + if (is_logical_device(device)) /* RAID */ > + rc = scsi_add_device(h->scsi_host, device->bus, > device->target, device->lun); > + else /* HBA */ > + rc = hpsa_add_sas_device(h->sas_host, device); > + > return rc; > } > > @@ -1706,21 +1723,23 @@ static void hpsa_remove_device(struct ctlr_info *h, > { > struct scsi_device *sdev = NULL; > > - sdev = scsi_device_lookup(h->scsi_host, device->bus, > + if (is_logical_device(device)) { /* RAID */ > + sdev = scsi_device_lookup(h->scsi_host, device->bus, > device->target, device->lun); > - > - if (sdev) { > - scsi_remove_device(sdev); > - scsi_device_put(sdev); > - } else { > - /* > - * We don't expect to get here. Future commands > - * to this device will get a selection timeout as > - * if the device were gone. > - */ > - hpsa_show_dev_msg(KERN_WARNING, h, device, > + if (sdev) { > + scsi_remove_device(sdev); > + scsi_device_put(sdev); > + } else { > + /* > + * We don't expect to get here. Future commands > + * to this device will get a selection timeout as > + * if the device were gone. > + */ > + hpsa_show_dev_msg(KERN_WARNING, h, device, > "didn't find device for removal."); > - } > + } > + } else /* HBA */ > + hpsa_remove_sas_device(device); > } > > static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno, > @@ -1915,11 +1934,24 @@ static int hpsa_slave_alloc(struct scsi_device *sdev) > > h = sdev_to_hba(sdev); > spin_lock_irqsave(&h->devlock, flags); > - sd = lookup_hpsa_scsi_dev(h, sdev_channel(sdev), > - sdev_id(sdev), sdev->lun); > - if (likely(sd)) { > + if (sdev_channel(sdev) == HPSA_PHYSICAL_DEVICE_BUS) { > + struct scsi_target *starget; > + struct sas_rphy *rphy; > + > + starget = scsi_target(sdev); > + rphy = target_to_rphy(starget); > + sd = hpsa_find_device_by_sas_rphy(h, rphy); > + if (sd) { > + sd->target = sdev_id(sdev); > + sd->lun = sdev->lun; > + } > + } else > + sd = lookup_hpsa_scsi_dev(h, sdev_channel(sdev), > + sdev_id(sdev), sdev->lun); > + > + if (sd && sd->expose_device) { > atomic_set(&sd->ioaccel_cmds_out, 0); > - sdev->hostda
[PATCH] pm80xx: remove the SCSI host before detaching from SAS transport
Previously, when this module was unloaded via 'rmmod' with at least one drive attached, the SCSI error handler thread would become stuck in an infinite recovery loop and lockup the system, necessitating a reboot. Once the SAS layer is detached, the driver will fail any subsequent commands since the target devices are removed. However, removing the SCSI host generates a SYNCHRONIZE CACHE (10) command, which was failed and left the error handler no method of recovery. This patch simply removes the SCSI host first so that no more commands can come down, prior to cleaning up the SAS layer. Note that the stack is built up with the SCSI host first, and then the SAS layer. Perhaps it should be reversed for symmetry, so that commands cannot be sent to the pm80xx driver prior to attaching the SAS layer? What was really strange about this bug was that it was introduced at commit cff549e4860f ("[SCSI]: proper state checking and module refcount handling in scsi_device_get"). This commit appears to tinker with how the reference counting is performed for SCSI device objects. My theory is that prior to this commit, the refcount for a device object was blindly incremented at some point during the teardown process which coincidentially made the device stick around during the procedure, which also coincidentially made any commands sent to the driver not fail (since the device was technically still "there"). After this commit was applied, my theory is the refcount for the device object is not being incremented at a specific point anymore, which makes the device go away, and thus made the pm80xx driver fail any subsequent commands. You may also want to see the following for more details: [1] http://www.spinics.net/lists/linux-scsi/msg37208.html [2] http://marc.info/?l=linux-scsi&m=144416476406993&w=2 Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index bdc624f..b1b2dd7 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -1096,10 +1096,10 @@ static void pm8001_pci_remove(struct pci_dev *pdev) struct pm8001_hba_info *pm8001_ha; int i, j; pm8001_ha = sha->lldd_ha; + scsi_remove_host(pm8001_ha->shost); sas_unregister_ha(sha); sas_remove_host(pm8001_ha->shost); list_del(&pm8001_ha->list); - scsi_remove_host(pm8001_ha->shost); PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF); PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha); -- 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: [PATCH v4.3-rc7] be2iscsi : Fix bogus WARN_ON length check
Tim: Reviewed-by: Manoj Kumar --- Manoj Kumar On 10/30/2015 1:22 PM, tim.gard...@canonical.com wrote: From: Tim Gardner drivers/scsi/be2iscsi/be_main.c: In function 'be_sgl_create_contiguous': drivers/scsi/be2iscsi/be_main.c:3187:18: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] WARN_ON(!length > 0); gcc version 5.2.1 Cc: Jayamohan Kallickal Cc: Minh Tran Cc: John Soni Jose Cc: "James E.J. Bottomley" Signed-off-by: Tim Gardner --- drivers/scsi/be2iscsi/be_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 7a6dbfb..5cdcd29 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -3184,7 +3184,7 @@ be_sgl_create_contiguous(void *virtual_address, { WARN_ON(!virtual_address); WARN_ON(!physical_address); - WARN_ON(!length > 0); + WARN_ON(!length); WARN_ON(!sgl); sgl->va = virtual_address; -- 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 v4.3-rc7] be2iscsi : Fix bogus WARN_ON length check
From: Tim Gardner drivers/scsi/be2iscsi/be_main.c: In function 'be_sgl_create_contiguous': drivers/scsi/be2iscsi/be_main.c:3187:18: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses] WARN_ON(!length > 0); gcc version 5.2.1 Cc: Jayamohan Kallickal Cc: Minh Tran Cc: John Soni Jose Cc: "James E.J. Bottomley" Signed-off-by: Tim Gardner --- drivers/scsi/be2iscsi/be_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index 7a6dbfb..5cdcd29 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -3184,7 +3184,7 @@ be_sgl_create_contiguous(void *virtual_address, { WARN_ON(!virtual_address); WARN_ON(!physical_address); - WARN_ON(!length > 0); + WARN_ON(!length); WARN_ON(!sgl); sgl->va = virtual_address; -- 1.9.1 -- 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] sd: Clear PS bit before Mode Select.
According to SPC-4, in a Mode Select, the PS bit in Mode Pages is reserved and must be set to 0 by the driver. In the sd implementation, function cache_type_store does a Mode Sense, which might set the PS bit on the read buffer, followed by a Mode Select, which receives the same buffer, without explicitly clearing the PS bit. So, in cases where target supports saving the Mode Page to a non-volatile location, we end up doing a Mode Select with the PS bit set, which could cause an illegal request error if the target is checking this. This was observed on a new firmware change, which was subsequently reverted, but this changes sd.c to be more compliant with SPC-4. This patch clears the PS bit in the buffer returned by Mode Select, right before it is used in the Mode Select command. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/sd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 3f37022..f724777 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -204,6 +204,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr, buffer_data[2] &= ~0x05; buffer_data[2] |= wce << 2 | rcd; sp = buffer_data[0] & 0x80 ? 1 : 0; + buffer_data[0] &= ~0x80; if (scsi_mode_select(sdp, 1, sp, 8, buffer_data, len, SD_TIMEOUT, SD_MAX_RETRIES, &data, &sshdr)) { -- 2.1.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
Re: [PATCH 2/5] ipr: Clear NO_ULEN_CHK bit when resource is a vset.
On 10/30/2015 11:49 AM, Gabriel Krisman Bertazi wrote: if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { - if (scsi_cmd->underflow == 0) + if (scsi_cmd->underflow == 0 && !ipr_is_vset_device(res)) This section is getting quite convoluted. If there isn't really that much common between ipr_is_gscsi(res) and ipr_is_vset_device(res) anymore, it would read much better as distinct segments: if (ipr_is_gscsi(res)) ... if (ipr_is_vset_devices(res)) ... This will avoid having multiple calls to ipr_is_gscsi() and ipr_is_vset_device() in the same section of code. --- Manoj Kumar -- 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] megaraid_sas : Add locking to megasas_aen_polling
From: Glenn Watkins Under conditions of offlining drives, and rescanning the scsi host, we can get into situations that the megasas_aen_polling kthread can crash(GPF) in the megasas_aen_polling work queue: [ 1206.568641] general protection fault: [#1] SMP [ 1206.569479] Modules linked in: xt_tcpudp nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack iptable_filter ip_tables x_tables coretemp crct10dif_pclmul crc32_pclmul aesni_intel ablk_helper cryptd psmouse lrw vmwgfx gf128mul serio_raw glue_helper aes_x86_64 ppdev ttm microcode vmw_balloon drm_kms_helper drm parport_pc parport fb_sys_fops sysimgblt sysfillrect syscopyarea vmw_vmci binfmt_misc floppy mptspi mptscsih vmw_pvscsi megaraid_sas pata_acpi mptbase vmxnet3 [ 1206.576488] CPU: 0 PID: 1157 Comm: kworker/0:2 Not tainted 4.3.0-rc7-svt1 #1 [ 1206.577520] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/14/2014 [ 1206.579101] Workqueue: events megasas_aen_polling [megaraid_sas] [ 1206.580007] task: 8818bb7b8000 ti: 8818ca28 task.ti: 8818ca28 [ 1206.581104] RIP: 0010:[] [] bdi_unregister+0x3d/0x1e0 [ 1206.582339] RSP: 0018:8818ca283cb8 EFLAGS: 00010246 [ 1206.583131] RAX: dead0200 RBX: 8818bb603f08 RCX: 8818c6487800 [ 1206.584184] RDX: 8818bb603f08 RSI: 7fff RDI: 81f9aa68 [ 1206.585243] RBP: 8818ca283d18 R08: R09: [ 1206.586294] R10: 000fffe0 R11: dead0200 R12: 8818bb6042f0 [ 1206.587346] R13: 8818bb604530 R14: 00ae R15: 0080 [ 1206.588388] FS: () GS:88193fc0() knlGS: [ 1206.589598] CS: 0010 DS: ES: CR0: 8005003b [ 1206.590457] CR2: 01a89000 CR3: 0018c07f2000 CR4: 000406f0 [ 1206.591545] Stack: [ 1206.591870] 8818bb6042f0 8818bb603d78 00ae 0080 [ 1206.593098] 8818ca283ce8 8108f683 8818ca283d18 813332b0 [ 1206.594308] 8818ca283d18 8818bb603d78 8818bb6042f0 8818bb604530 [ 1206.595532] Call Trace: [ 1206.595922] [] ? cancel_delayed_work_sync+0x13/0x20 [ 1206.596903] [] ? blk_sync_queue+0x80/0x90 [ 1206.597753] [] blk_cleanup_queue+0x114/0x150 [ 1206.598645] [] __scsi_remove_device+0x54/0xd0 [ 1206.599556] [] scsi_remove_device+0x2f/0x50 [ 1206.600441] [] megasas_aen_polling+0x34d/0x670 [megaraid_sas] [ 1206.601561] [] process_one_work+0x14c/0x400 [ 1206.602449] [] worker_thread+0x117/0x480 [ 1206.603295] [] ? create_worker+0x1c0/0x1c0 [ 1206.604160] [] kthread+0xc9/0xe0 [ 1206.604898] [] ? flush_kthread_worker+0x90/0x90 [ 1206.605831] [] ret_from_fork+0x3f/0x70 [ 1206.606659] [] ? flush_kthread_worker+0x90/0x90 [ 1206.607585] Code: c7 c7 68 aa f9 81 48 83 ec 48 e8 bf 76 59 00 48 8b 43 08 48 8b 13 49 bb 00 02 00 00 00 00 ad de 48 c7 c7 68 aa f9 81 48 89 42 08 <48> 89 10 4c 89 5b 08 e8 27 76 59 00 e8 32 92 f4 ff 48 8d 7b 50 [ 1206.611938] RIP [] bdi_unregister+0x3d/0x1e0 [ 1206.612856] RSP This can be readily reproduced by a pair of shell scripts - one of which loops on onlining / offlining drives via MegaCli (or storcli, if you prefer) #!/bin/bash while [ 1 ]; do /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:0] a0 &>2 /opt/MegaRAID/MegaCli/MegaCli64 pdoffline physdrv[32:11] a0 &>2 /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:0] a0 &>2 /opt/MegaRAID/MegaCli/MegaCli64 pdonline physdrv[32:11] a0 &>2 done Meanwhile, the second script is looping on rescanning the scsi hosts: #!/bin/bash while [ 1 ]; do for (( l=0; l<4; l++ )); do echo - - - > /sys/class/scsi_host/host$l/scan done done This was originally introduced in the following commit: commit 7e8a75f4dfbff173977b2f58799c3eceb7b09afd Author: Yang, Bo Date: Tue Oct 6 14:50:17 2009 -0600 [SCSI] megaraid_sas: Add the support for updating the OS after adding/removing the devices from FW The fix for this is to add some locking around the AEN polling. Since this affects all kernels since 2.6.33, I have also CC'ed the stable list. Signed-off-by: Glenn Watkins Signed-off-by: Ben Guthro --- drivers/scsi/megaraid/megaraid_sas_base.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index eaa81e5..d203d9d 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -6640,6 +6640,7 @@ megasas_aen_polling(struct work_struct *work) if (doscan) { dev_info(&instance->pdev->dev, "scanning for scsi%d...\n", instance->host->host_no); + mutex_lock(&host->scan_mutex); if (megasas_get_pd_list(instance) == 0) { for (i = 0; i < MEGASAS_MAX_PD_CHANNELS; i++) {
Re: [PATCH 1 23/25] hpsa: fix multiple issues in path_info_show
Reviewed-by: Matthew R. Ochs -- 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 25/32] scsi: hisi_sas: add abnormal irq handler
On 30/10/2015 14:10, Arnd Bergmann wrote: On Monday 26 October 2015 22:14:56 John Garry wrote: Add abnormal irq handler. This handler is concerned with phy down event. Also add port formed and port deformed handlers. Signed-off-by: John Garry I noticed a couple more coding style issues in this patch than elsewhere, so here is a slightly more detailed review. +static void hisi_sas_port_notify_formed(struct asd_sas_phy *sas_phy, int lock) +{ + struct sas_ha_struct *sas_ha = sas_phy->ha; + struct hisi_hba *hisi_hba = NULL; + int i = 0; + struct hisi_sas_phy *phy = sas_phy->lldd_phy; + struct asd_sas_port *sas_port = sas_phy->port; + struct hisi_sas_port *port; + unsigned long flags = 0; Here and in general, please avoid initializing local variables to zero, as that prevents gcc from warning about uses that come before the real initialization. The flags that get passed into spin_lock_irqsave() are architecture specific, so you cannot rely on '0' to have a particular meaning. Noted. Will change. + if (!sas_port) + return; + + while (sas_ha->sas_phy[i]) { Using a for() loop would avoid the initialization here. Yes. + if (sas_ha->sas_phy[i] == sas_phy) { + hisi_hba = (struct hisi_hba *)sas_ha->lldd_ha; lldd_ha is a void pointer, so you don't need a cast. Noted. + port = &hisi_hba->port[i]; + break; + } + i++; + } The loop is really odd, as you apparently only try to find the array index to a pointer you already have. Is there no space for driver-specific data in 'asd_sas_phy'? If there is, just point to per-phy structure that you define yourself and put the index into that structure. I believe you already have a struct like that. This code is a remnant from when we used to have the driver designed in such a way that we had a single Scsi Host and which had multiple cores (and hisi_hba's). Will address. + if (hisi_hba == NULL) { When checking a pointer for validity, do not compare against NULL, but write this as 'if (!hisi_hba)', which is the more normal coding style. OK. + pr_err("%s: could not find hba\n", __func__); + return; + } Better use dev_err() to print the device name, but remove the __func__ argument. Again, when you have the per-phy structure, put a pointer to the device in there. + + if (lock) + spin_lock_irqsave(&hisi_hba->lock, flags); + port->port_attached = 1; + port->id = phy->port_id; + phy->port = port; + sas_port->lldd_port = port; + + if (lock) + spin_unlock_irqrestore(&hisi_hba->lock, flags); +} This breaks some checking tools that try to validate the uses of locks. Better wrap the function in another one depending on the caller. When using spinlocks in general, it's also better to replace the "I have no clue where I'm called from" spin_lock_irqsave() call with either spin_lock() or spin_lock_irq() if at all possible. Ok, I can remove the lock check. I know that the function can be called from irq context in my irq handler and also from multiple functions in libsas (whose context I am unsure of). Arnd -- 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 . Thanks for the detailed review. I'll try and address your comments for other patches. john -- 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 22/25] hpsa: enhance device messages
Reviewed-by: Matthew R. Ochs -- 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 1/5] ipr: Add delay to ensure coherent dumps.
Add a holding pattern prior to collecting dump data, to wait for the IOA indication that the Mailbox register is stable and won't change without an explicit reset. This ensures we'll be collecting meaningful dump data, even when dumping right after an adapter reset. In the event of a timeout, we still force the dump, since a partial dump still might be useful. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 51 +++ drivers/scsi/ipr.h | 3 +++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index b62836d..4034cd3 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -8277,6 +8277,42 @@ static int ipr_reset_get_unit_check_job(struct ipr_cmnd *ipr_cmd) return IPR_RC_JOB_RETURN; } +static int ipr_dump_mailbox_wait(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + + ENTER; + + if (ioa_cfg->sdt_state != GET_DUMP) + return IPR_RC_JOB_RETURN; + + if (!ioa_cfg->sis64 || !ipr_cmd->u.time_left || + readl(ioa_cfg->regs.sense_interrupt_reg) & IPR_PCII_MAILBOX_STABLE) { + + if (!ipr_cmd->u.time_left) + dev_err(&ioa_cfg->pdev->dev, + "Timed out waiting for Mailbox register. " + "Dump might be truncated.\n"); + + ioa_cfg->sdt_state = READ_DUMP; + ioa_cfg->dump_timeout = 0; + if (ioa_cfg->sis64) + ipr_reset_start_timer(ipr_cmd, IPR_SIS64_DUMP_TIMEOUT); + else + ipr_reset_start_timer(ipr_cmd, IPR_SIS32_DUMP_TIMEOUT); + ipr_cmd->job_step = ipr_reset_wait_for_dump; + schedule_work(&ioa_cfg->work_q); + + } else { + ipr_cmd->u.time_left -= IPR_CHECK_FOR_RESET_TIMEOUT; + ipr_reset_start_timer(ipr_cmd, + IPR_CHECK_FOR_RESET_TIMEOUT); + } + + LEAVE; + return IPR_RC_JOB_RETURN; +} + /** * ipr_reset_restore_cfg_space - Restore PCI config space. * @ipr_cmd: ipr command struct @@ -8326,20 +8362,11 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd) if (ioa_cfg->in_ioa_bringdown) { ipr_cmd->job_step = ipr_ioa_bringdown_done; + } else if (ioa_cfg->sdt_state == GET_DUMP) { + ipr_cmd->job_step = ipr_dump_mailbox_wait; + ipr_cmd->u.time_left = IPR_WAIT_FOR_MAILBOX; } else { ipr_cmd->job_step = ipr_reset_enable_ioa; - - if (GET_DUMP == ioa_cfg->sdt_state) { - ioa_cfg->sdt_state = READ_DUMP; - ioa_cfg->dump_timeout = 0; - if (ioa_cfg->sis64) - ipr_reset_start_timer(ipr_cmd, IPR_SIS64_DUMP_TIMEOUT); - else - ipr_reset_start_timer(ipr_cmd, IPR_SIS32_DUMP_TIMEOUT); - ipr_cmd->job_step = ipr_reset_wait_for_dump; - schedule_work(&ioa_cfg->work_q); - return IPR_RC_JOB_RETURN; - } } LEAVE; diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index e4fb17a..69257c4 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -279,6 +279,9 @@ #define IPR_IPL_INIT_STAGE_TIME_MASK 0x #define IPR_PCII_IPL_STAGE_CHANGE (0x8000 >> 0) +#define IPR_PCII_MAILBOX_STABLE(0x8000 >> 4) +#define IPR_WAIT_FOR_MAILBOX (2 * HZ) + #define IPR_PCII_IOA_TRANS_TO_OPER (0x8000 >> 0) #define IPR_PCII_IOARCB_XFER_FAILED(0x8000 >> 3) #define IPR_PCII_IOA_UNIT_CHECKED (0x8000 >> 4) -- 2.1.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 2/5] ipr: Clear NO_ULEN_CHK bit when resource is a vset.
According to the IPR specification, Inhibit Underlength Checking bit must be disabled when sending commands to vsets. Enabling this bit for vset might cause SCSI commands to fail with an Illegal Request. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 4034cd3..da957b3 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -6364,7 +6364,7 @@ static int ipr_queuecommand(struct Scsi_Host *shost, ipr_cmd->done = ipr_scsi_eh_done; if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { - if (scsi_cmd->underflow == 0) + if (scsi_cmd->underflow == 0 && !ipr_is_vset_device(res)) ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_ULEN_CHK; ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_LINK_DESC; -- 2.1.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 3/5] ipr: Inquiry IOA page 0xC4 during initialization.
Add an IOA Inquiry command for Page 0xC4 during IOA initialization to collect cache capabilities, particularly to check if Sync IOA Write Cache is supported. Inquiry will happen right after Cap Inquiry on page 0xD0; and will execute only if the "Supported Pages" field in Inquiry Page 0x0 shows support for Page 0xC4. Otherwise, assume Sync IOA Write Cache is not supported. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 35 ++- drivers/scsi/ipr.h | 11 +++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index da957b3..a9f825a 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7722,6 +7722,39 @@ static int ipr_inquiry_page_supported(struct ipr_inquiry_page0 *page0, u8 page) } /** + * ipr_ioafp_pageC4_inquiry - Send a Page 0xC4 Inquiry to the adapter. + * @ipr_cmd: ipr command struct + * + * This function sends a Page 0xC4 inquiry to the adapter + * to retrieve software VPD information. + * + * Return value: + * IPR_RC_JOB_CONTINUE / IPR_RC_JOB_RETURN + **/ +static int ipr_ioafp_pageC4_inquiry(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + struct ipr_inquiry_page0 *page0 = &ioa_cfg->vpd_cbs->page0_data; + struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; + ENTER; + + ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + memset(pageC4, 0, sizeof(*pageC4)); + + if (ipr_inquiry_page_supported(page0, 0xC4)) { + ipr_ioafp_inquiry(ipr_cmd, 1, 0xC4, + (ioa_cfg->vpd_cbs_dma + + offsetof(struct ipr_misc_cbs, + pageC4_data)), + sizeof(struct ipr_inquiry_pageC4)); + return IPR_RC_JOB_RETURN; + } + + LEAVE; + return IPR_RC_JOB_CONTINUE; +} + +/** * ipr_ioafp_cap_inquiry - Send a Page 0xD0 Inquiry to the adapter. * @ipr_cmd: ipr command struct * @@ -7738,7 +7771,7 @@ static int ipr_ioafp_cap_inquiry(struct ipr_cmnd *ipr_cmd) struct ipr_inquiry_cap *cap = &ioa_cfg->vpd_cbs->cap; ENTER; - ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + ipr_cmd->job_step = ipr_ioafp_pageC4_inquiry; memset(cap, 0, sizeof(*cap)); if (ipr_inquiry_page_supported(page0, 0xD0)) { diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index 69257c4..f0238cc 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -849,6 +849,16 @@ struct ipr_inquiry_page0 { u8 page[IPR_INQUIRY_PAGE0_ENTRIES]; }__attribute__((packed)); +struct ipr_inquiry_pageC4 { + u8 peri_qual_dev_type; + u8 page_code; + u8 reserved1; + u8 len; + u8 cache_cap[4]; +#define IPR_CAP_SYNC_CACHE 0x08 + u8 reserved2[20]; +}__attribute__((packed)); + struct ipr_hostrcb_device_data_entry { struct ipr_vpd vpd; struct ipr_res_addr dev_res_addr; @@ -1322,6 +1332,7 @@ struct ipr_misc_cbs { struct ipr_inquiry_page0 page0_data; struct ipr_inquiry_page3 page3_data; struct ipr_inquiry_cap cap; + struct ipr_inquiry_pageC4 pageC4_data; struct ipr_mode_pages mode_pages; struct ipr_supported_device supp_dev; }; -- 2.1.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 4/5] ipr: Issue Configure Cache Parameters command.
Some new adapters require a special Configure Cache Parameters command to enable the adapter write cache, so send this during the adapter initialization if the adapter requires it. Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 59 +- drivers/scsi/ipr.h | 4 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index a9f825a..c83afeb 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -7671,6 +7671,63 @@ static int ipr_ioafp_query_ioa_cfg(struct ipr_cmnd *ipr_cmd) return IPR_RC_JOB_RETURN; } +static int ipr_ioa_service_action_failed(struct ipr_cmnd *ipr_cmd) +{ + u32 ioasc = be32_to_cpu(ipr_cmd->s.ioasa.hdr.ioasc); + + if (ioasc == IPR_IOASC_IR_INVALID_REQ_TYPE_OR_PKT) + return IPR_RC_JOB_CONTINUE; + + return ipr_reset_cmd_failed(ipr_cmd); +} + +static void ipr_build_ioa_service_action(struct ipr_cmnd *ipr_cmd, +__be32 res_handle, u8 sa_code) +{ + struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb; + + ioarcb->res_handle = res_handle; + ioarcb->cmd_pkt.cdb[0] = IPR_IOA_SERVICE_ACTION; + ioarcb->cmd_pkt.cdb[1] = sa_code; + ioarcb->cmd_pkt.request_type = IPR_RQTYPE_IOACMD; +} + +/** + * ipr_ioafp_set_caching_parameters - Issue Set Cache parameters service + * action + * + * Return value: + * none + **/ +static int ipr_ioafp_set_caching_parameters(struct ipr_cmnd *ipr_cmd) +{ + struct ipr_ioarcb *ioarcb = &ipr_cmd->ioarcb; + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; + + ENTER; + + ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + + if (pageC4->cache_cap[0] & IPR_CAP_SYNC_CACHE) { + ipr_build_ioa_service_action(ipr_cmd, +cpu_to_be32(IPR_IOA_RES_HANDLE), +IPR_IOA_SA_CHANGE_CACHE_PARAMS); + + ioarcb->cmd_pkt.cdb[2] = 0x40; + + ipr_cmd->job_step_failed = ipr_ioa_service_action_failed; + ipr_do_req(ipr_cmd, ipr_reset_ioa_job, ipr_timeout, + IPR_SET_SUP_DEVICE_TIMEOUT); + + LEAVE; + return IPR_RC_JOB_RETURN; + } + + LEAVE; + return IPR_RC_JOB_CONTINUE; +} + /** * ipr_ioafp_inquiry - Send an Inquiry to the adapter. * @ipr_cmd: ipr command struct @@ -7738,7 +7795,7 @@ static int ipr_ioafp_pageC4_inquiry(struct ipr_cmnd *ipr_cmd) struct ipr_inquiry_pageC4 *pageC4 = &ioa_cfg->vpd_cbs->pageC4_data; ENTER; - ipr_cmd->job_step = ipr_ioafp_query_ioa_cfg; + ipr_cmd->job_step = ipr_ioafp_set_caching_parameters; memset(pageC4, 0, sizeof(*pageC4)); if (ipr_inquiry_page_supported(page0, 0xC4)) { diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index f0238cc..aab5c6d 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -216,6 +216,10 @@ #define IPR_SET_ALL_SUPPORTED_DEVICES 0x80 #define IPR_IOA_SHUTDOWN 0xF7 #defineIPR_WR_BUF_DOWNLOAD_AND_SAVE0x05 +#define IPR_IOA_SERVICE_ACTION 0xD2 + +/* IOA Service Actions */ +#define IPR_IOA_SA_CHANGE_CACHE_PARAMS 0x14 /* * Timeouts -- 2.1.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 5/5] ipr: Driver version 2.6.3.
Signed-off-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index aab5c6d..ce49ca9 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -39,8 +39,8 @@ /* * Literals */ -#define IPR_DRIVER_VERSION "2.6.2" -#define IPR_DRIVER_DATE "(June 11, 2015)" +#define IPR_DRIVER_VERSION "2.6.3" +#define IPR_DRIVER_DATE "(October 17, 2015)" /* * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding -- 2.1.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
Re: [PATCH v2 27/32] scsi: hisi_sas: add smp protocol support
On 30/10/2015 13:53, Arnd Bergmann wrote: On Monday 26 October 2015 22:14:58 John Garry wrote: + /* + * DMA-map SMP request, response buffers + */ + /* req */ + sg_req = &task->smp_task.smp_req; + elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE); + if (!elem) + return -ENOMEM; + req_len = sg_dma_len(sg_req); + req_dma_addr = sg_dma_address(sg_req); If you only use the first element, could you just use dma_map_single()? Can do. Actually sg_req seems only ever has one element: expander.c, smp_execute_task() sg_init_one(&task->smp_task.smp_req, req, req_size); + hdr->cmd_table_addr_lo = cpu_to_le32(lower_32_bits(req_dma_addr)); + hdr->cmd_table_addr_hi = cpu_to_le32(upper_32_bits(req_dma_addr)); + + hdr->sts_buffer_addr_lo = + cpu_to_le32(lower_32_bits(slot->status_buffer_dma)); + hdr->sts_buffer_addr_hi = + cpu_to_le32(upper_32_bits(slot->status_buffer_dma)); + I see these a lot in your code. Could you replace this wit hdr->cmd_table_addr = cpu_to_le64(req_dma_addr); This seems reasonable. They are not swapped. and so on? That would be much more readable. Or are the two __le32 variables swapped? If so, you could add a small helper function like static inline __le64 cpu_to_le64_wordswapped(u64 val) { return cpu_to_le64(val >> 32 | val << 32); } Arnd . cheers -- 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 21/25] hpsa: disable report lun data caching
> On Oct 28, 2015, at 5:06 PM, Don Brace wrote: > > From: Scott Teel > > When external target arrays are present, disable the firmware's > normal behavior of returning a cached copy of the report lun data, > and force it to collect new data each time we request a report luns. > > This is necessary for external arrays, since there may be no > reliable signal from the external array to the smart array when > lun configuration changes, and thus when driver requests > report luns, it may be stale data. > > Use diag options to turn off RPL data caching. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 86 +++ > drivers/scsi/hpsa_cmd.h |3 ++ > 2 files changed, 89 insertions(+) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index e521acd..33fd0aa 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct > ctlr_info *h, > static void hpsa_command_resubmit_worker(struct work_struct *work); > static u32 lockup_detected(struct ctlr_info *h); > static int detect_controller_lockup(struct ctlr_info *h); > +static void hpsa_disable_rld_caching(struct ctlr_info *h); > static int hpsa_luns_changed(struct ctlr_info *h); > > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > @@ -6386,6 +6387,24 @@ static int fill_cmd(struct CommandList *c, u8 cmd, > struct ctlr_info *h, > c->Request.CDB[8] = (size >> 8) & 0xFF; > c->Request.CDB[9] = size & 0xFF; > break; > + case BMIC_SENSE_DIAG_OPTIONS: > + c->Request.CDBLen = 16; > + c->Request.type_attr_dir = > + TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ); > + c->Request.Timeout = 0; > + /* Spec says this should be BMIC_WRITE */ > + c->Request.CDB[0] = BMIC_READ; > + c->Request.CDB[6] = BMIC_SENSE_DIAG_OPTIONS; > + break; > + case BMIC_SET_DIAG_OPTIONS: > + c->Request.CDBLen = 16; > + c->Request.type_attr_dir = > + TYPE_ATTR_DIR(cmd_type, > + ATTR_SIMPLE, XFER_WRITE); > + c->Request.Timeout = 0; > + c->Request.CDB[0] = BMIC_WRITE; > + c->Request.CDB[6] = BMIC_SET_DIAG_OPTIONS; > + break; > case HPSA_CACHE_FLUSH: > c->Request.CDBLen = 12; > c->Request.type_attr_dir = > @@ -8086,6 +8105,7 @@ static void hpsa_rescan_ctlr_worker(struct work_struct > *work) > hpsa_scan_start(h->scsi_host); > scsi_host_put(h->scsi_host); > } else if (h->discovery_polling) { > + hpsa_disable_rld_caching(h); > if (hpsa_luns_changed(h)) { > struct Scsi_Host *sh = NULL; > > @@ -8423,6 +8443,72 @@ out: > kfree(flush_buf); > } > > +/* Make controller gather fresh report lun data each time we > + * send down a report luns request > + */ > +static void hpsa_disable_rld_caching(struct ctlr_info *h) > +{ > + u32 *options; > + struct CommandList *c; > + int rc; > + > + /* Don't bother trying to set diag options if locked up */ > + if (unlikely(h->lockup_detected)) > + return; > + > + options = kzalloc(sizeof(*options), GFP_KERNEL); > + if (!options) { > + dev_err(&h->pdev->dev, > + "Error: failed to disable rld caching, during > alloc.\n"); > + return; > + } > + > + c = cmd_alloc(h); > + > + /* first, get the current diag options settings */ > + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, > + PCI_DMA_FROMDEVICE, NO_TIMEOUT); > + if ((rc != 0) || (c->err_info->CommandStatus != 0)) > + goto errout; > + > + /* Now, set the bit for disabling the RLD caching */ > + *options |= HPSA_DIAG_OPTS_DISABLE_RLD_CACHING; > + > + if (fill_cmd(c, BMIC_SET_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, > + PCI_DMA_TODEVICE, NO_TIMEOUT); > + if ((rc != 0) || (c->err_info->CommandStatus != 0)) > + goto errout; > + > + /* Now verify that it got set: */ > + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h,
Re: [PATCH 1 20/25] hpsa: add discovery polling for PT RAID devices.
> On Oct 30, 2015, at 9:08 AM, Don Brace wrote: > > On 10/29/2015 03:59 PM, Matthew R. Ochs wrote: >>> On Oct 29, 2015, at 3:51 PM, Don Brace wrote: >>> On 10/29/2015 03:20 PM, Matthew R. Ochs wrote: > On Oct 28, 2015, at 5:06 PM, Don Brace > wrote: > > From: Scott Teel > > > > There are problems with getting configuration change notification > in pass-through RAID environments. So, activate flag > h->discovery_polling when one of these devices is detected in > update_scsi_devices. > > After discovery_polling is set, execute a report luns from > rescan_controller_worker (every 30 seconds). > > If the data from report_luns is different than last > time (binary compare), execute a full rescan via update_scsi_devices. > > Reviewed-by: Scott Teel > > > Reviewed-by: Justin Lindley > > > Reviewed-by: Kevin Barnett > > > Signed-off-by: Don Brace > > > --- > drivers/scsi/hpsa.c | 68 > +++ > drivers/scsi/hpsa.h |2 ++ > 2 files changed, 70 insertions(+) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 8d67648..e521acd 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct > ctlr_info *h, > static void hpsa_command_resubmit_worker(struct work_struct *work); > static u32 lockup_detected(struct ctlr_info *h); > static int detect_controller_lockup(struct ctlr_info *h); > +static int hpsa_luns_changed(struct ctlr_info *h); > > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > { > @@ -3904,6 +3905,18 @@ static void hpsa_update_scsi_devices(struct > ctlr_info *h, int hostno) > hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes); > this_device = currentsd[ncurrent]; > > + /* Turn on discovery_polling if there are ext target devices. > + * Event-based change notification is unreliable for those. > + */ > + if (!h->discovery_polling) { > + if (tmpdevice->external) { > + h->discovery_polling = 1; > + dev_info(&h->pdev->dev, > + "External target, activate discovery > polling.\n"); > + } > + } > + > + > *this_device = *tmpdevice; > this_device->physical_device = physical_device; > > @@ -8022,6 +8035,41 @@ static int hpsa_offline_devices_ready(struct > ctlr_info *h) > return 0; > } > > +static int hpsa_luns_changed(struct ctlr_info *h) > +{ > + int rc = 1; /* assume there are changes */ > + struct ReportLUNdata *logdev = NULL; > + > + /* if we can't find out if lun data has changed, > + * assume that it has. > + */ > + > + if (!h->lastlogicals) > + goto out; > + > + logdev = kzalloc(sizeof(*logdev), GFP_KERNEL); > + if (!logdev) { > + dev_warn(&h->pdev->dev, > + "Out of memory, can't track lun changes.\n"); > + goto out; > + } > + if (hpsa_scsi_do_report_luns(h, 1, logdev, sizeof(*logdev), 0)) { > + dev_warn(&h->pdev->dev, > + "report luns failed, can't track lun changes.\n"); > + goto out; > + } > + if (memcmp(logdev, h->lastlogicals, sizeof(*logdev))) { > + dev_info(&h->pdev->dev, > + "Lun changes detected.\n"); > + memcpy(h->lastlogicals, logdev, sizeof(*logdev)); > + goto out; > + } else > + rc = 0; /* no changes detected. */ > +out: > + kfree(logdev); > + return rc; > +} > + > static void hpsa_rescan_ctlr_worker(struct work_struct *work) > { > unsigned long flags; > @@ -8037,6 +8085,18 @@ static void hpsa_rescan_ctlr_worker(struct > work_struct *work) > hpsa_ack_ctlr_events(h); > hpsa_scan_start(h->scsi_host); > scsi_host_put(h->scsi_host); > + } else if (h->discovery_polling) { > + if (hpsa_luns_changed(h)) { > + struct Scsi_Host *sh = NULL; > + > + dev_info(&h->pdev->dev, > + "driver discovery polling rescan.\n"); > + sh = scsi_host_get(h->scsi_host); > + if (sh != NULL) { > + hpsa_scan_start(sh); > + scsi_host_put(sh); > + } > + } > } > spin_lock_irqsave(&h->lock, flags); > if (!h->remove_in_progress) > @@ -8277,6 +8337,8 @@ reinit_after_soft_reset: > > /* En
Re: [PATCH 1 17/25] hpsa: move scsi_add_device and scsi_remove_device calls to new function
> On Oct 29, 2015, at 3:30 PM, Don Brace wrote: > > On 10/29/2015 12:21 PM, Matthew R. Ochs wrote: >>> On Oct 28, 2015, at 5:06 PM, Don Brace wrote: >>> >>> From: Kevin Barnett >>> >>> preparation for adding the sas transport class >>> >>> Reviewed-by: Scott Teel >>> Reviewed-by: Justin Lindley >>> Reviewed-by: Kevin Barnett >>> Signed-off-by: Don Brace >>> --- >>> drivers/scsi/hpsa.c | 65 >>> +++ >>> 1 file changed, 39 insertions(+), 26 deletions(-) >>> >>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c >>> index 24b3c8c..06207e2 100644 >>> --- a/drivers/scsi/hpsa.c >>> +++ b/drivers/scsi/hpsa.c >>> @@ -1660,6 +1660,37 @@ static void >>> hpsa_update_log_drive_phys_drive_ptrs(struct ctlr_info *h, >>> } >>> } >>> >>> +static int hpsa_add_device(struct ctlr_info *h, struct hpsa_scsi_dev_t >>> *device) >>> +{ >>> + int rc = 0; >>> + >>> + rc = scsi_add_device(h->scsi_host, device->bus, >>> + device->target, device->lun); >>> + return rc; >>> +} >>> + >>> +static void hpsa_remove_device(struct ctlr_info *h, >>> + struct hpsa_scsi_dev_t *device) >>> +{ >>> + struct scsi_device *sdev = NULL; >>> + >>> + sdev = scsi_device_lookup(h->scsi_host, device->bus, >>> + device->target, device->lun); >>> + >>> + if (sdev) { >>> + scsi_remove_device(sdev); >>> + scsi_device_put(sdev); >>> + } else { >>> + /* >>> +* We don't expect to get here. Future commands >>> +* to this device will get a selection timeout as >>> +* if the device were gone. >>> +*/ >>> + hpsa_show_dev_msg(KERN_WARNING, h, device, >>> + "didn't find device for removal."); >>> + } >>> +} >>> + >>> static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno, >>> struct hpsa_scsi_dev_t *sd[], int nsds) >>> { >>> @@ -1672,7 +1703,6 @@ static void adjust_hpsa_scsi_table(struct ctlr_info >>> *h, int hostno, >>> unsigned long flags; >>> struct hpsa_scsi_dev_t **added, **removed; >>> int nadded, nremoved; >>> - struct Scsi_Host *sh = NULL; >>> >>> /* >>> * A reset can cause a device status to change >>> @@ -1792,46 +1822,29 @@ static void adjust_hpsa_scsi_table(struct ctlr_info >>> *h, int hostno, >>> if (hostno == -1 || !changes) >>> goto free_and_out; >>> >>> - sh = h->scsi_host; >>> - if (sh == NULL) { >>> - dev_warn(&h->pdev->dev, "%s: scsi_host is null\n", __func__); >>> - return; >>> - } >> Are we guaranteed that h->scsi_host will never be NULL when running in here? >> Or when >> the newly introduced hpsa_remove_device() is invoked elsewhere for that >> matter? >> >> This commit loses this check and scsi_device_lookup() is not tolerant of a >> NULL >> scsi_host * (first action is to grab the host_lock). > Better to be safe. I'll add a check in both functions. With these changes Reviewed-by: Matthew R. Ochs -- 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: Blk-mq/scsi-mq Tuning
On 10/30/2015 03:12 PM, Chad Dupuis wrote: > > > On Fri, 30 Oct 2015, Hannes Reinecke wrote: > >> On 10/30/2015 02:25 PM, Chad Dupuis wrote: >>> >>> >>> On Fri, 30 Oct 2015, Hannes Reinecke wrote: >>> On 10/28/2015 09:11 PM, Chad Dupuis wrote: > Hi Folks, > > We¹ve begun to explore blk-mq and scsi-mq and wanted to know if there > were > any best practices in terms of block layer settings. We¹re looking > specifically at the FCoE and iSCSI protocols. > > A little background on the queues in our hardware first: we have a per > connection transmit queue and multiple, global receive queues. The > transmit queues are not pegged to a particular CPU. The receive > queues > are pegged to the first N CPUs where N is the number of receive > queues. > We set the nr_hw_queues in the scsi_host_template to N as well. > Weelll ... I think you'll run into issues here. The whole point of the multiqueue implementation is that you can tag the submission _and_ completion queue to a single CPU, thereby eliminating locking. If you only peg the completion queue to a CPU you'll still have contention on the submission queue, needing to take locks etc. Plus you will _inevitably_ incur cache misses, as the completion will basically never occur on the same CPU which did the submissoin. Hence the context needs to be bounced to the CPU holding the completion queue, or you'll need to do a IPI to inform the submitting CPU. But if you do that you're essentially doing single-queue submission, so I doubt we're seeing that great improvements. >>> >>> This was why I was asking if there was a blk-mq API to be able to set >>> CPU affinity for the hardware context queues so I could steer the >>> submissions to the CPUs that my receive queues are on (even if they are >>> allowed to float). >>> >> But what would that achieve? >> Each of the hardware context queues would still having to use the >> same submission queue, so you'd have to have some serialisation >> with spinlocks et.al. during submission. Which is what blk-mq >> tries to avoid. >> Am I wrong? > > Sadly, no I believe you're correct. So essentially the upshot seems to > be if you can have a 1x1 request:response queue then sticking with the > older queuecommand method is better? > Hmm; you might be getting some performance improvements as the submission path from the blocklayer down is more efficient, but in your case the positive effects might be eliminated by reducing the number of receive queues. But then you never know until you try :-) The alternative would indeed be to move to MC/S with block-mq; that should give you some benefits as you'd be able to utilize several queues. I have actually discussed that with Emulex; moving to MC/S in the iSCSI stack might indeed be viable when using blk-mq. It would be a rather good match with the existing blk-mq implementation, and most of the implementation would be in the iSCSI stack, reducing the burden on the driver vendors :-) 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
[PATCH 2/8] pm80xx: add support for PMC Sierra 8070 and PMC Sierra 8072 SAS controllers
These SAS controllers support speeds up to 12Gb. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_defs.h | 2 ++ drivers/scsi/pm8001/pm8001_init.c | 4 +++- drivers/scsi/pm8001/pm8001_sas.h | 4 +++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_defs.h b/drivers/scsi/pm8001/pm8001_defs.h index f14ec6e..199527d 100644 --- a/drivers/scsi/pm8001/pm8001_defs.h +++ b/drivers/scsi/pm8001/pm8001_defs.h @@ -51,6 +51,8 @@ enum chip_flavors { chip_8076, chip_8077, chip_8006, + chip_8070, + chip_8072 }; enum phy_speed { diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index 8c094fd..2106ac3 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -58,6 +58,8 @@ static const struct pm8001_chip_info pm8001_chips[] = { [chip_8076] = {0, 16, &pm8001_80xx_dispatch,}, [chip_8077] = {0, 16, &pm8001_80xx_dispatch,}, [chip_8006] = {0, 16, &pm8001_80xx_dispatch,}, + [chip_8070] = {0, 8, &pm8001_80xx_dispatch,}, + [chip_8072] = {0, 16, &pm8001_80xx_dispatch,}, }; static int pm8001_id; @@ -1234,7 +1236,7 @@ MODULE_AUTHOR("Anand Kumar Santhanam "); MODULE_AUTHOR("Sangeetha Gnanasekaran "); MODULE_AUTHOR("Nikith Ganigarakoppal "); MODULE_DESCRIPTION( - "PMC-Sierra PM8001/8006/8081/8088/8089/8074/8076/8077 " + "PMC-Sierra PM8001/8006/8081/8088/8089/8074/8076/8077/8070/8072 " "SAS/SATA controller driver"); MODULE_VERSION(DRV_VERSION); MODULE_LICENSE("GPL"); diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h index e2e97db..9fa9705 100644 --- a/drivers/scsi/pm8001/pm8001_sas.h +++ b/drivers/scsi/pm8001/pm8001_sas.h @@ -106,7 +106,9 @@ do {\ #define DEV_IS_EXPANDER(type) ((type == SAS_EDGE_EXPANDER_DEVICE) || (type == SAS_FANOUT_EXPANDER_DEVICE)) #define IS_SPCV_12G(dev) ((dev->device == 0X8074)\ || (dev->device == 0X8076) \ - || (dev->device == 0X8077)) + || (dev->device == 0X8077) \ + || (dev->device == 0X8070) \ + || (dev->device == 0X8072)) #define PM8001_NAME_LENGTH 32/* generic length of strings */ extern struct list_head hba_list; -- 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
[PATCH 4/8] pm80xx: add support for ATTO devices during SAS address initiailization
ATTO SAS controllers retrieve the SAS address from the NVRAM in a location different from non-ATTO PMC Sierra SAS controllers. This patch makes the necessary adjustments in order to retrieve the SAS address on these types of adapters. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index feaf504..fdbfab6 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -636,6 +636,11 @@ static void pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha) payload.minor_function = 0; payload.length = 128; } + } else if ((pm8001_ha->chip_id == chip_8070 || + pm8001_ha->chip_id == chip_8072) && + pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) { + payload.minor_function = 4; + payload.length = 4096; } else { payload.minor_function = 1; payload.length = 4096; @@ -662,6 +667,11 @@ static void pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha) else if (deviceid == 0x0042) pm8001_ha->sas_addr[j] = payload.func_specific[0x010 + i]; + } else if ((pm8001_ha->chip_id == chip_8070 || + pm8001_ha->chip_id == chip_8072) && + pm8001_ha->pdev->subsystem_vendor == PCI_VENDOR_ID_ATTO) { + pm8001_ha->sas_addr[j] = + payload.func_specific[0x010 + i]; } else pm8001_ha->sas_addr[j] = payload.func_specific[0x804 + i]; -- 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
[PATCH 5/8] pm80xx: set PHY profiles for ATTO 12Gb SAS controllers
PHY profiles are not saved in NVRAM on ATTO 12Gb SAS controllers. Therefore, in order for the controller to function in a wide range of configurations, the PHY profiles must be statically set. This patch provides the necessary functionality to do so. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 130 ++ drivers/scsi/pm8001/pm8001_sas.h | 2 + drivers/scsi/pm8001/pm80xx_hwi.c | 32 ++ 3 files changed, 164 insertions(+) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index fdbfab6..a0e55d4 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -732,6 +732,131 @@ static int pm8001_get_phy_settings_info(struct pm8001_hba_info *pm8001_ha) return 0; } +struct pm8001_mpi3_phy_pg_trx_config { + u32 LaneLosCfg; + u32 LanePgaCfg1; + u32 LanePisoCfg1; + u32 LanePisoCfg2; + u32 LanePisoCfg3; + u32 LanePisoCfg4; + u32 LanePisoCfg5; + u32 LanePisoCfg6; + u32 LaneBctCtrl; +}; + +/** + * pm8001_get_internal_phy_settings : Retrieves the internal PHY settings + * @pm8001_ha : our adapter + * @phycfg : PHY config page to populate + */ +static +void pm8001_get_internal_phy_settings(struct pm8001_hba_info *pm8001_ha, + struct pm8001_mpi3_phy_pg_trx_config *phycfg) +{ + phycfg->LaneLosCfg = 0x0132; + phycfg->LanePgaCfg1 = 0x00203949; + phycfg->LanePisoCfg1 = 0x00FF; + phycfg->LanePisoCfg2 = 0xFF01; + phycfg->LanePisoCfg3 = 0xE7011300; + phycfg->LanePisoCfg4 = 0x631C40C0; + phycfg->LanePisoCfg5 = 0xF8102036; + phycfg->LanePisoCfg6 = 0xF74A1000; + phycfg->LaneBctCtrl = 0x00FB33F8; +} + +/** + * pm8001_get_external_phy_settings : Retrieves the external PHY settings + * @pm8001_ha : our adapter + * @phycfg : PHY config page to populate + */ +static +void pm8001_get_external_phy_settings(struct pm8001_hba_info *pm8001_ha, + struct pm8001_mpi3_phy_pg_trx_config *phycfg) +{ + phycfg->LaneLosCfg = 0x0132; + phycfg->LanePgaCfg1 = 0x00203949; + phycfg->LanePisoCfg1 = 0x00FF; + phycfg->LanePisoCfg2 = 0xFF01; + phycfg->LanePisoCfg3 = 0xE7011300; + phycfg->LanePisoCfg4 = 0x63349140; + phycfg->LanePisoCfg5 = 0xF8102036; + phycfg->LanePisoCfg6 = 0xF80D9300; + phycfg->LaneBctCtrl = 0x00FB33F8; +} + +/** + * pm8001_get_phy_mask : Retrieves the mask that denotes if a PHY is int/ext + * @pm8001_ha : our adapter + * @phymask : The PHY mask + */ +static +void pm8001_get_phy_mask(struct pm8001_hba_info *pm8001_ha, int *phymask) +{ + switch (pm8001_ha->pdev->subsystem_device) { + case 0x0070: /* H1280 - 8 external 0 internal */ + case 0x0072: /* H12F0 - 16 external 0 internal */ + *phymask = 0x; + break; + + case 0x0071: /* H1208 - 0 external 8 internal */ + case 0x0073: /* H120F - 0 external 16 internal */ + *phymask = 0x; + break; + + case 0x0080: /* H1244 - 4 external 4 internal */ + *phymask = 0x00F0; + break; + + case 0x0081: /* H1248 - 4 external 8 internal */ + *phymask = 0x0FF0; + break; + + case 0x0082: /* H1288 - 8 external 8 internal */ + *phymask = 0xFF00; + break; + + default: + PM8001_INIT_DBG(pm8001_ha, + pm8001_printk("Unknown subsystem device=0x%.04x", + pm8001_ha->pdev->subsystem_device)); + } +} + +/** + * pm8001_set_phy_settings_ven_117c_12Gb : Configure ATTO 12Gb PHY settings + * @pm8001_ha : our adapter + */ +static +int pm8001_set_phy_settings_ven_117c_12G(struct pm8001_hba_info *pm8001_ha) +{ + struct pm8001_mpi3_phy_pg_trx_config phycfg_int; + struct pm8001_mpi3_phy_pg_trx_config phycfg_ext; + int phymask = 0; + int i = 0; + + memset(&phycfg_int, 0, sizeof(phycfg_int)); + memset(&phycfg_ext, 0, sizeof(phycfg_ext)); + + pm8001_get_internal_phy_settings(pm8001_ha, &phycfg_int); + pm8001_get_external_phy_settings(pm8001_ha, &phycfg_ext); + pm8001_get_phy_mask(pm8001_ha, &phymask); + + for (i = 0; i < pm8001_ha->chip->n_phy; i++) { + if (phymask & (1 << i)) {/* Internal PHY */ + pm8001_set_phy_profile_single(pm8001_ha, i, + sizeof(phycfg_int) / sizeof(u32), + (u32 *)&phycfg_int); + + } else { /* External PHY */ + pm8001_set_phy_profile_single(pm8001_ha, i, + sizeof(phycfg_ext) / sizeof(u32), + (u32 *)&phycfg_ext); + } + } + + return 0; +} + /** * pm8001_configure_phy_settings : C
[PATCH 6/8] pm80xx: do not examine registers for iButton feature if ATTO adapter
ATTO adapters do not support this feature. If the firmware fails to be ready, it should not check the examined registers in order to examine the state of the feature in order to prevent undefined behavior. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm80xx_hwi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 29c548b..eb4fee6 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -1267,6 +1267,8 @@ pm80xx_chip_soft_rst(struct pm8001_hba_info *pm8001_ha) /* check iButton feature support for motherboard controller */ if (pm8001_ha->pdev->subsystem_vendor != PCI_VENDOR_ID_ADAPTEC2 && + pm8001_ha->pdev->subsystem_vendor != + PCI_VENDOR_ID_ATTO && pm8001_ha->pdev->subsystem_vendor != 0) { ibutton0 = pm8001_cr32(pm8001_ha, 0, MSGU_HOST_SCRATCH_PAD_6); -- 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
[PATCH 8/8] pm80xx: avoid a panic if MSI(X) interrupts are disabled
If MSI(X) interrupts are disabled via the kernel command line (pci=nomsi), the pm8001 driver will kernel panic because it does not detect that MSI interrupts are disabled and will soldier on and attempt to configure MSI interrupts anyways. This leads to a kernel panic, most likely because a required data structure is not available down the line. Using the pci_msi_enabled() function in order to detect if MSI interrupts are enabled before configuring them resolves this issue and avoids a kernel panic when the module is loaded. Additionally, the irq_vector structure must be initialized when legacy interrupts are being used otherwise legacy interrupts will simply not function and result in another panic. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index ab99984..bdc624f 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -482,7 +482,8 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev, #ifdef PM8001_USE_TASKLET /* Tasklet for non msi-x interrupt handler */ - if ((!pdev->msix_cap) || (pm8001_ha->chip_id == chip_8001)) + if ((!pdev->msix_cap || !pci_msi_enabled()) + || (pm8001_ha->chip_id == chip_8001)) tasklet_init(&pm8001_ha->tasklet[0], pm8001_tasklet, (unsigned long)&(pm8001_ha->irq_vector[0])); else @@ -951,7 +952,7 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha) pdev = pm8001_ha->pdev; #ifdef PM8001_USE_MSIX - if (pdev->msix_cap) + if (pdev->msix_cap && pci_msi_enabled()) return pm8001_setup_msix(pm8001_ha); else { PM8001_INIT_DBG(pm8001_ha, @@ -962,6 +963,8 @@ static u32 pm8001_request_irq(struct pm8001_hba_info *pm8001_ha) intx: /* initialize the INT-X interrupt */ + pm8001_ha->irq_vector[0].irq_id = 0; + pm8001_ha->irq_vector[0].drv_inst = pm8001_ha; rc = request_irq(pdev->irq, pm8001_interrupt_handler_intx, IRQF_SHARED, DRV_NAME, SHOST_TO_SAS_HA(pm8001_ha->shost)); return rc; @@ -1112,7 +1115,8 @@ static void pm8001_pci_remove(struct pci_dev *pdev) #endif #ifdef PM8001_USE_TASKLET /* For non-msix and msix interrupts */ - if ((!pdev->msix_cap) || (pm8001_ha->chip_id == chip_8001)) + if ((!pdev->msix_cap || !pci_msi_enabled()) || + (pm8001_ha->chip_id == chip_8001)) tasklet_kill(&pm8001_ha->tasklet[0]); else for (j = 0; j < PM8001_MAX_MSIX_VEC; j++) @@ -1161,7 +1165,8 @@ static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state) #endif #ifdef PM8001_USE_TASKLET /* For non-msix and msix interrupts */ - if ((!pdev->msix_cap) || (pm8001_ha->chip_id == chip_8001)) + if ((!pdev->msix_cap || !pci_msi_enabled()) || + (pm8001_ha->chip_id == chip_8001)) tasklet_kill(&pm8001_ha->tasklet[0]); else for (j = 0; j < PM8001_MAX_MSIX_VEC; j++) @@ -1231,7 +1236,8 @@ static int pm8001_pci_resume(struct pci_dev *pdev) goto err_out_disable; #ifdef PM8001_USE_TASKLET /* Tasklet for non msi-x interrupt handler */ - if ((!pdev->msix_cap) || (pm8001_ha->chip_id == chip_8001)) + if ((!pdev->msix_cap || !pci_msi_enabled()) || + (pm8001_ha->chip_id == chip_8001)) tasklet_init(&pm8001_ha->tasklet[0], pm8001_tasklet, (unsigned long)&(pm8001_ha->irq_vector[0])); else -- 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
[PATCH 3/8] pm80xx: add ATTO PCI IDs to pm8001_pci_table
These PCI IDs allow the pm8001 driver to load against ATTO 12Gb SAS controllers that use PMC Sierra 8070 and PMC Sierra 8072 SAS chips. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index 2106ac3..feaf504 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -1181,6 +1181,20 @@ static struct pci_device_id pm8001_pci_table[] = { PCI_VENDOR_ID_ADAPTEC2, 0x0808, 0, 0, chip_8077 }, { PCI_VENDOR_ID_ADAPTEC2, 0x8074, PCI_VENDOR_ID_ADAPTEC2, 0x0404, 0, 0, chip_8074 }, + { PCI_VENDOR_ID_ATTO, 0x8070, + PCI_VENDOR_ID_ATTO, 0x0070, 0, 0, chip_8070 }, + { PCI_VENDOR_ID_ATTO, 0x8070, + PCI_VENDOR_ID_ATTO, 0x0071, 0, 0, chip_8070 }, + { PCI_VENDOR_ID_ATTO, 0x8072, + PCI_VENDOR_ID_ATTO, 0x0072, 0, 0, chip_8072 }, + { PCI_VENDOR_ID_ATTO, 0x8072, + PCI_VENDOR_ID_ATTO, 0x0073, 0, 0, chip_8072 }, + { PCI_VENDOR_ID_ATTO, 0x8070, + PCI_VENDOR_ID_ATTO, 0x0080, 0, 0, chip_8070 }, + { PCI_VENDOR_ID_ATTO, 0x8072, + PCI_VENDOR_ID_ATTO, 0x0081, 0, 0, chip_8072 }, + { PCI_VENDOR_ID_ATTO, 0x8072, + PCI_VENDOR_ID_ATTO, 0x0082, 0, 0, chip_8072 }, {} /* terminate list */ }; -- 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
[PATCH 1/8] pm80xx: configure PHY settings based on subsystem vendor ID
Previuosly, all PMC Sierra 80xx controllers are assumed to be a motherboard controller, except if the subsystem vendor ID was equal to PCI_VENDOR_ID_ADAPTEC. The driver then attempts to load PHY settings from NVRAM. While this may be correct behavior for most controllers, it does not work with Adaptec and ATTO controllers since they do not store PHY settings in NVRAM and choose to use either custom PHY settings or chip defaults. Loading random values from NVRAM may cause the controllers to malfunction in this edge case. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index 5c0356f..8c094fd 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -720,6 +720,23 @@ static int pm8001_get_phy_settings_info(struct pm8001_hba_info *pm8001_ha) return 0; } +/** + * pm8001_configure_phy_settings : Configures PHY settings based on vendor ID. + * @pm8001_ha : our hba. + */ +static int pm8001_configure_phy_settings(struct pm8001_hba_info *pm8001_ha) +{ + switch (pm8001_ha->pdev->subsystem_vendor) { + case PCI_VENDOR_ID_ATTO: + case PCI_VENDOR_ID_ADAPTEC2: + case 0: + return 0; + + default: + return pm8001_get_phy_settings_info(pm8001_ha); + } +} + #ifdef PM8001_USE_MSIX /** * pm8001_setup_msix - enable MSI-X interrupt @@ -902,12 +919,9 @@ static int pm8001_pci_probe(struct pci_dev *pdev, pm8001_init_sas_add(pm8001_ha); /* phy setting support for motherboard controller */ - if (pdev->subsystem_vendor != PCI_VENDOR_ID_ADAPTEC2 && - pdev->subsystem_vendor != 0) { - rc = pm8001_get_phy_settings_info(pm8001_ha); - if (rc) - goto err_out_shost; - } + if (pm8001_configure_phy_settings(pm8001_ha)) + goto err_out_shost; + pm8001_post_sas_ha_init(shost, chip); rc = sas_register_ha(SHOST_TO_SAS_HA(shost)); if (rc) -- 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
[PATCH 7/8] pm80xx: wait a minimum of 500ms before issuing commands to SPCv
The documentation for the 8070 and 8072 SPCv chip explicitly states that a minimum of 500ms must elapse before issuing commands, otherwise the SPCv may not process them and the firmware may get into an unrecoverable state requiring a reboot. While the Linux guys will probably think this is 'racy', it is called out in the chip documentation and inserting this delay makes power management function properly. Signed-off-by: Benjamin Rood --- drivers/scsi/pm8001/pm8001_init.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index a0e55d4..ab99984 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -1190,6 +1190,7 @@ static int pm8001_pci_resume(struct pci_dev *pdev) int rc; u8 i = 0, j; u32 device_state; + u32 wait_count; DECLARE_COMPLETION_ONSTACK(completion); pm8001_ha = sha->lldd_ha; device_state = pdev->current_state; @@ -1243,6 +1244,17 @@ static int pm8001_pci_resume(struct pci_dev *pdev) for (i = 1; i < pm8001_ha->number_of_intr; i++) PM8001_CHIP_DISP->interrupt_enable(pm8001_ha, i); } + + if (pm8001_ha->chip_id == chip_8070 || + pm8001_ha->chip_id == chip_8072) { + wait_count = 500; + do { + mdelay(1); + } while (--wait_count); + } + + /* Spin up the PHYs */ + pm8001_ha->flags = PM8001F_RUN_TIME; for (i = 0; i < pm8001_ha->chip->n_phy; i++) { pm8001_ha->phy[i].enable_completion = &completion; -- 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
[PATCH 0/8] pm80xx: Add ATTO 12Gb HBA support and fix various issues
The following series of patches are primarily aimed at adding support for ATTO's 12Gb SAS adapters, which are based off of the PMC-Sierra 8070/8072 series chips. I have also addressed other various issues that were discovered during our testing and verification phase, mainly with resume-from-sleep, configuring PHY profiles, and using legacy interrupts. Diffstat: drivers/scsi/pm8001/pm8001_defs.h | 2 ++ drivers/scsi/pm8001/pm8001_init.c | 212 +++ drivers/scsi/pm8001/pm8001_sas.h | 6 +++- drivers/scsi/pm8001/pm80xx_hwi.c | 34 ++ 4 files changed, 241 insertions(+), 13 deletions(-) -- 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 25/25] hpsa: bump the driver version
On 28.10.2015 23:07, Don Brace wrote: > Reviewed-by: Justin Lindley > Reviewed-by: Scott Teel > Reviewed-by: Kevin Barnett > Reviewed-by: Gerry Morong > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index ca38a00..1b43157 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -56,7 +56,7 @@ > #include "hpsa.h" > > /* HPSA_DRIVER_VERSION must be 3 byte values (0-255) separated by '.' */ Is the description^ still valid ? Reviewed-by: Tomas Henzl Tomas > -#define HPSA_DRIVER_VERSION "3.4.10-0" > +#define HPSA_DRIVER_VERSION "3.4.14-0" > #define DRIVER_NAME "HP HPSA Driver (v " HPSA_DRIVER_VERSION ")" > #define HPSA "hpsa" > > > -- > 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 1 24/25] hpsa: add in sas transport class
On 28.10.2015 23:06, Don Brace wrote: > From: Kevin Barnett > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace Reviewed-by: Tomas Henzl Tomas -- 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 23/25] hpsa: fix multiple issues in path_info_show
On 28.10.2015 23:06, Don Brace wrote: > From: Rasmus Villemoes > > path_info_show() seems to be broken in multiple ways. > > First, there's > > 817 return snprintf(buf, output_len+1, "%s%s%s%s%s%s%s%s", > 818 path[0], path[1], path[2], path[3], > 819 path[4], path[5], path[6], path[7]); > > so hopefully output_len contains the combined length of the eight > strings. Otherwise, snprintf will stop copying to the output > buffer, but still end up reporting that combined length - which > in turn would result in user-space getting a bunch of useless nul > bytes (thankfully the upper sysfs layer seems to clear the output > buffer before passing it to the various ->show routines). But we have > > 767 output_len = snprintf(path[i], > 768 PATH_STRING_LEN, "[%d:%d:%d:%d] %20.20s ", > 769 h->scsi_host->host_no, > 770 hdev->bus, hdev->target, hdev->lun, > 771 scsi_device_type(hdev->devtype)); > > so output_len at best contains the length of the last string printed. > > Inside the loop, we then otherwise add to output_len. By magic, > we still have PATH_STRING_LEN available every time... This > wouldn't really be a problem if the bean-counting has been done > properly and each line actually does fit in 50 bytes, and maybe > it does, but I don't immediately see why. Suppose we end up > taking this branch: > > 802 output_len += snprintf(path[i] + output_len, > 803 PATH_STRING_LEN, > 804 "BOX: %hhu BAY: %hhu %s\n", > 805 box, bay, active); > > An optimistic estimate says this uses strlen("BOX: 1 BAY: 2 > Active\n") which is 21. Now add the 20 bytes guaranteed by the > %20.20s and then some for the rest of that format string, and > we're easily over 50 bytes. I don't think we can get over 100 > bytes even being pessimistic, so this just means we'll scribble > into the next path[i+1] and maybe get that overwritten later, > leading to some garbled output (in fact, since we'd overwrite the > previous string's 0-terminator, we could end up with one very > long string and then print various suffixes of that, leading to > much more than 400 bytes of output). Except of course when we're > filling path[7], where overrunning it means writing random stuff > to the kernel stack, which is usually a lot of fun. > > We can fix all of that and get rid of the 400 byte stack buffer by > simply writing directly to the given output buffer, which the upper > layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where > it is writing to, so this doesn't make the spin lock hold time any > longer. Using scnprintf ensures that output_len always represents the > number of bytes actually written to the buffer, so we'll report the > proper amount to the upper layer. > > Signed-off-by: Rasmus Villemoes > Signed-off-by: Don Brace Reviewed-by: Tomas Henzl Tomas -- 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 22/25] hpsa: enhance device messages
On 28.10.2015 23:06, Don Brace wrote: > Reviewed-by: Justin Lindley > Reviewed-by: Scott Teel > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 45 - > 1 file changed, 40 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 33fd0aa..b2418c3 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -609,7 +609,7 @@ static inline int is_logical_dev_addr_mode(unsigned char > scsi3addr[]) > } > > static const char * const raid_label[] = { "0", "4", "1(+0)", "5", "5+1", > "6", > - "1(+0)ADM", "UNKNOWN" > + "1(+0)ADM", "UNKNOWN", "PHYS DRV" > }; > #define HPSA_RAID_0 0 > #define HPSA_RAID_4 1 > @@ -618,7 +618,8 @@ static const char * const raid_label[] = { "0", "4", > "1(+0)", "5", "5+1", "6", > #define HPSA_RAID_51 4 > #define HPSA_RAID_6 5 /* also used for RAID 60 */ > #define HPSA_RAID_ADM6 /* also used for RAID 1+0 ADM */ > -#define RAID_UNKNOWN (ARRAY_SIZE(raid_label) - 1) > +#define RAID_UNKNOWN (ARRAY_SIZE(raid_label) - 2) > +#define PHYSICAL_DRIVE (ARRAY_SIZE(raid_label) - 1) RAID_UNKNOWN is used in few other places - raid_level_show for example, shouldn't be that also adapted? -tm > / > > static inline bool is_logical_device(struct hpsa_scsi_dev_t *device) > { > @@ -1144,21 +1145,55 @@ static int hpsa_find_target_lun(struct ctlr_info *h, > static void hpsa_show_dev_msg(const char *level, struct ctlr_info *h, > struct hpsa_scsi_dev_t *dev, char *description) > { > +#define LABEL_SIZE 25 > + char label[LABEL_SIZE]; > + > if (dev == NULL) > return; > > if (h == NULL || h->pdev == NULL || h->scsi_host == NULL) > return; > > + switch (dev->devtype) { > + case TYPE_RAID: > + snprintf(label, LABEL_SIZE, "controller"); > + break; > + case TYPE_ENCLOSURE: > + snprintf(label, LABEL_SIZE, "enclosure"); > + break; > + case TYPE_DISK: > + if (dev->external) > + snprintf(label, LABEL_SIZE, "external"); > + else if (!is_logical_dev_addr_mode(dev->scsi3addr)) > + snprintf(label, LABEL_SIZE, "%s", > + raid_label[PHYSICAL_DRIVE]); > + else > + snprintf(label, LABEL_SIZE, "RAID-%s", > + dev->raid_level > RAID_UNKNOWN ? "?" : > + raid_label[dev->raid_level]); > + break; > + case TYPE_ROM: > + snprintf(label, LABEL_SIZE, "rom"); > + break; > + case TYPE_TAPE: > + snprintf(label, LABEL_SIZE, "tape"); > + break; > + case TYPE_MEDIUM_CHANGER: > + snprintf(label, LABEL_SIZE, "changer"); > + break; > + default: > + snprintf(label, LABEL_SIZE, "UNKNOWN"); > + break; > + } > + > dev_printk(level, &h->pdev->dev, > - "scsi %d:%d:%d:%d: %s %s %.8s %.16s RAID-%s > SSDSmartPathCap%c En%c Exp=%d\n", > + "scsi %d:%d:%d:%d: %s %s %.8s %.16s %s > SSDSmartPathCap%c En%c Exp=%d\n", > h->scsi_host->host_no, dev->bus, dev->target, dev->lun, > description, > scsi_device_type(dev->devtype), > dev->vendor, > dev->model, > - dev->raid_level > RAID_UNKNOWN ? > - "RAID-?" : raid_label[dev->raid_level], > + label, > dev->offload_config ? '+' : '-', > dev->offload_enabled ? '+' : '-', > dev->expose_device); > > -- > 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 1 21/25] hpsa: disable report lun data caching
On 28.10.2015 23:06, Don Brace wrote: > From: Scott Teel > > When external target arrays are present, disable the firmware's > normal behavior of returning a cached copy of the report lun data, > and force it to collect new data each time we request a report luns. > > This is necessary for external arrays, since there may be no > reliable signal from the external array to the smart array when > lun configuration changes, and thus when driver requests > report luns, it may be stale data. > > Use diag options to turn off RPL data caching. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 86 > +++ > drivers/scsi/hpsa_cmd.h |3 ++ > 2 files changed, 89 insertions(+) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index e521acd..33fd0aa 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct > ctlr_info *h, > static void hpsa_command_resubmit_worker(struct work_struct *work); > static u32 lockup_detected(struct ctlr_info *h); > static int detect_controller_lockup(struct ctlr_info *h); > +static void hpsa_disable_rld_caching(struct ctlr_info *h); > static int hpsa_luns_changed(struct ctlr_info *h); > > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > @@ -6386,6 +6387,24 @@ static int fill_cmd(struct CommandList *c, u8 cmd, > struct ctlr_info *h, > c->Request.CDB[8] = (size >> 8) & 0xFF; > c->Request.CDB[9] = size & 0xFF; > break; > + case BMIC_SENSE_DIAG_OPTIONS: > + c->Request.CDBLen = 16; > + c->Request.type_attr_dir = > + TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ); > + c->Request.Timeout = 0; > + /* Spec says this should be BMIC_WRITE */ > + c->Request.CDB[0] = BMIC_READ; > + c->Request.CDB[6] = BMIC_SENSE_DIAG_OPTIONS; > + break; > + case BMIC_SET_DIAG_OPTIONS: > + c->Request.CDBLen = 16; > + c->Request.type_attr_dir = > + TYPE_ATTR_DIR(cmd_type, > + ATTR_SIMPLE, XFER_WRITE); > + c->Request.Timeout = 0; > + c->Request.CDB[0] = BMIC_WRITE; > + c->Request.CDB[6] = BMIC_SET_DIAG_OPTIONS; > + break; > case HPSA_CACHE_FLUSH: > c->Request.CDBLen = 12; > c->Request.type_attr_dir = > @@ -8086,6 +8105,7 @@ static void hpsa_rescan_ctlr_worker(struct work_struct > *work) > hpsa_scan_start(h->scsi_host); > scsi_host_put(h->scsi_host); > } else if (h->discovery_polling) { > + hpsa_disable_rld_caching(h); > if (hpsa_luns_changed(h)) { > struct Scsi_Host *sh = NULL; > > @@ -8423,6 +8443,72 @@ out: > kfree(flush_buf); > } > > +/* Make controller gather fresh report lun data each time we > + * send down a report luns request > + */ > +static void hpsa_disable_rld_caching(struct ctlr_info *h) > +{ > + u32 *options; > + struct CommandList *c; > + int rc; > + > + /* Don't bother trying to set diag options if locked up */ > + if (unlikely(h->lockup_detected)) > + return; > + > + options = kzalloc(sizeof(*options), GFP_KERNEL); > + if (!options) { > + dev_err(&h->pdev->dev, > + "Error: failed to disable rld caching, during > alloc.\n"); > + return; > + } > + > + c = cmd_alloc(h); > + > + /* first, get the current diag options settings */ > + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, > + PCI_DMA_FROMDEVICE, NO_TIMEOUT); > + if ((rc != 0) || (c->err_info->CommandStatus != 0)) > + goto errout; > + > + /* Now, set the bit for disabling the RLD caching */ > + *options |= HPSA_DIAG_OPTS_DISABLE_RLD_CACHING; > + > + if (fill_cmd(c, BMIC_SET_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, > + PCI_DMA_TODEVICE, NO_TIMEOUT); > + if ((rc != 0) || (c->err_info->CommandStatus != 0)) > + goto errout; > + > + /* Now verify that it got set: */ > + if (fill_cmd(c, BMIC_SENSE_DIAG_OPTIONS, h, options, 4, 0, > + RAID_CTLR_LUNID, TYPE_CMD)) > + goto errout; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, >
Re: [PATCH 1 03/25] hpsa: check for null arguments to dev_printk
On 10/30/2015 02:47 AM, Hannes Reinecke wrote: On 10/28/2015 11:04 PM, Don Brace wrote: Check for NULLs. Signed-off-by: Don Brace --- drivers/scsi/hpsa.c |6 ++ drivers/scsi/hpsa.h |2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 864fb03..6b6e9bb 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1139,6 +1139,12 @@ static int hpsa_find_target_lun(struct ctlr_info *h, static inline void hpsa_show_dev_msg(const char *level, struct ctlr_info *h, struct hpsa_scsi_dev_t *dev, char *description) { + if (dev == NULL) + return; + + if (h == NULL || h->pdev == NULL || h->scsi_host == NULL) + return; + dev_printk(level, &h->pdev->dev, "scsi %d:%d:%d:%d: %s %s %.8s %.16s RAID-%s SSDSmartPathCap%c En%c Exp=%d\n", h->scsi_host->host_no, dev->bus, dev->target, dev->lun, diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 27debb3..d6c4ebf 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -34,7 +34,7 @@ struct access_method { }; struct hpsa_scsi_dev_t { - int devtype; + unsigned int devtype; int bus, target, lun; /* as presented to the OS */ unsigned char scsi3addr[8]; /* as presented to the HW */ #define RAID_CTLR_LUNID "\0\0\0\0\0\0\0\0" That's not really a check for NULL, isn't it? Should rather be moved into an individual patch. Cheers, Hannes Making the change. -- 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: Blk-mq/scsi-mq Tuning
On Fri, 30 Oct 2015, Hannes Reinecke wrote: On 10/30/2015 02:25 PM, Chad Dupuis wrote: On Fri, 30 Oct 2015, Hannes Reinecke wrote: On 10/28/2015 09:11 PM, Chad Dupuis wrote: Hi Folks, We¹ve begun to explore blk-mq and scsi-mq and wanted to know if there were any best practices in terms of block layer settings. We¹re looking specifically at the FCoE and iSCSI protocols. A little background on the queues in our hardware first: we have a per connection transmit queue and multiple, global receive queues. The transmit queues are not pegged to a particular CPU. The receive queues are pegged to the first N CPUs where N is the number of receive queues. We set the nr_hw_queues in the scsi_host_template to N as well. Weelll ... I think you'll run into issues here. The whole point of the multiqueue implementation is that you can tag the submission _and_ completion queue to a single CPU, thereby eliminating locking. If you only peg the completion queue to a CPU you'll still have contention on the submission queue, needing to take locks etc. Plus you will _inevitably_ incur cache misses, as the completion will basically never occur on the same CPU which did the submissoin. Hence the context needs to be bounced to the CPU holding the completion queue, or you'll need to do a IPI to inform the submitting CPU. But if you do that you're essentially doing single-queue submission, so I doubt we're seeing that great improvements. This was why I was asking if there was a blk-mq API to be able to set CPU affinity for the hardware context queues so I could steer the submissions to the CPUs that my receive queues are on (even if they are allowed to float). But what would that achieve? Each of the hardware context queues would still having to use the same submission queue, so you'd have to have some serialisation with spinlocks et.al. during submission. Which is what blk-mq tries to avoid. Am I wrong? Sadly, no I believe you're correct. So essentially the upshot seems to be if you can have a 1x1 request:response queue then sticking with the older queuecommand method is better? Cheers, Hannes
Re: [PATCH 1 18/25] External array LUNs must use target and lun numbers assigned by the
On 28.10.2015 23:06, Don Brace wrote: > From: Scott Teel > > external array. So the driver must treat these differently from > local LUNs when assigning lun/target. > > LUN's 'model' field has been used to detect Lun types that need > special treatment, but the desire is to eliminate the need to reference > specific array models, and support any external array. > > Pass-through RAID (PTRAID) luns are not luns of the local controller, > so they are not reported in LUN count of command 'ID controller'. > However, they ARE reported in "Report logical Luns" command. > Local luns are listed first, then PTRAID LUNs. > > The number of luns from "Report LUNs" in excess of those reported by > 'ID controller' are therefore the PTRAID LUNS. > > We can now remove function is_ext_target, and the 'white list' > array of supported model names. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 141 > ++- > drivers/scsi/hpsa.h |1 > drivers/scsi/hpsa_cmd.h | 11 > 3 files changed, 127 insertions(+), 26 deletions(-) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 06207e2..11ea3e5 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -276,7 +276,6 @@ static int hpsa_scsi_ioaccel_queue_command(struct > ctlr_info *h, > static void hpsa_command_resubmit_worker(struct work_struct *work); > static u32 lockup_detected(struct ctlr_info *h); > static int detect_controller_lockup(struct ctlr_info *h); > -static int is_ext_target(struct ctlr_info *h, struct hpsa_scsi_dev_t > *device); > > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > { > @@ -777,7 +776,7 @@ static ssize_t path_info_show(struct device *dev, > hdev->bus, hdev->target, hdev->lun, > scsi_device_type(hdev->devtype)); > > - if (is_ext_target(h, hdev) || > + if (hdev->external || > hdev->devtype == TYPE_RAID || > is_logical_device(hdev)) { > output_len += snprintf(path[i] + output_len, > @@ -3037,6 +3036,35 @@ out: > return rc; > } > > +static int hpsa_bmic_id_controller(struct ctlr_info *h, > + struct bmic_identify_controller *buf, size_t bufsize) > +{ > + int rc = IO_OK; > + struct CommandList *c; > + struct ErrorInfo *ei; > + > + c = cmd_alloc(h); > + > + rc = fill_cmd(c, BMIC_IDENTIFY_CONTROLLER, h, buf, bufsize, > + 0, RAID_CTLR_LUNID, TYPE_CMD); > + if (rc) > + goto out; > + > + rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, > + PCI_DMA_FROMDEVICE, NO_TIMEOUT); > + if (rc) > + goto out; > + ei = c->err_info; > + if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) { > + hpsa_scsi_interpret_error(h, c); > + rc = -1; > + } > +out: > + cmd_free(h, c); > + return rc; > +} > + > + > static int hpsa_bmic_id_physical_device(struct ctlr_info *h, > unsigned char scsi3addr[], u16 bmic_device_index, > struct bmic_identify_physical_device *buf, size_t bufsize) > @@ -3513,27 +3541,6 @@ static void hpsa_update_device_supports_aborts(struct > ctlr_info *h, > } > } > > -static unsigned char *ext_target_model[] = { > - "MSA2012", > - "MSA2024", > - "MSA2312", > - "MSA2324", > - "P2000 G3 SAS", > - "MSA 2040 SAS", > - NULL, > -}; > - > -static int is_ext_target(struct ctlr_info *h, struct hpsa_scsi_dev_t *device) > -{ > - int i; > - > - for (i = 0; ext_target_model[i]; i++) > - if (strncmp(device->model, ext_target_model[i], > - strlen(ext_target_model[i])) == 0) > - return 1; > - return 0; > -} > - > /* > * Helper function to assign bus, target, lun mapping of devices. > * Logical drive target and lun are assigned at this time, but > @@ -3557,7 +3564,7 @@ static void figure_bus_target_lun(struct ctlr_info *h, > return; > } > /* It's a logical device */ > - if (is_ext_target(h, device)) { > + if (device->external) { > hpsa_set_bus_target_lun(device, > HPSA_EXTERNAL_RAID_VOLUME_BUS, (lunid >> 16) & 0x3fff, > lunid & 0x00ff); > @@ -3591,7 +3598,7 @@ static int add_ext_target_dev(struct ctlr_info *h, > if (!is_logical_dev_addr_mode(lunaddrbytes)) > return 0; /* It's the logical targets that may lack lun 0. */ > > - if (!is_ext_target(h, tmpdevice)) > + if (!tmpdevice->external) > return 0; /* Only external target devices have this problem. */ > > if (tmpdevice->lun == 0) /* if lun is 0, then we have a lun 0. */ > @@ -3650,6 +3657,29 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_i
Re: [PATCH 1 19/25] hpsa: eliminate fake lun0 enclosures
On 28.10.2015 23:06, Don Brace wrote: > From: Scott Teel > > We don't need to create fake enclosure devices at Lun0 > in external target array configurations anymore. > This was done to support Pre-SCSI rev 5 controllers > that didn't suppoprt report luns commands, so the > SCSI layer had to scan targets. If there was no > LUN at LUN 0, then the target scan would stop, and > move to the next target. Lun0 enclosure device > was added to prevent sparsely-numbered LUNs from > being missed. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace Reviewed-by: Tomas Henzl Tomas -- 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 25/32] scsi: hisi_sas: add abnormal irq handler
On Monday 26 October 2015 22:14:56 John Garry wrote: > Add abnormal irq handler. This handler is concerned with > phy down event. > Also add port formed and port deformed handlers. > > Signed-off-by: John Garry I noticed a couple more coding style issues in this patch than elsewhere, so here is a slightly more detailed review. > +static void hisi_sas_port_notify_formed(struct asd_sas_phy *sas_phy, int > lock) > +{ > + struct sas_ha_struct *sas_ha = sas_phy->ha; > + struct hisi_hba *hisi_hba = NULL; > + int i = 0; > + struct hisi_sas_phy *phy = sas_phy->lldd_phy; > + struct asd_sas_port *sas_port = sas_phy->port; > + struct hisi_sas_port *port; > + unsigned long flags = 0; Here and in general, please avoid initializing local variables to zero, as that prevents gcc from warning about uses that come before the real initialization. The flags that get passed into spin_lock_irqsave() are architecture specific, so you cannot rely on '0' to have a particular meaning. > + if (!sas_port) > + return; > + > + while (sas_ha->sas_phy[i]) { Using a for() loop would avoid the initialization here. > + if (sas_ha->sas_phy[i] == sas_phy) { > + hisi_hba = (struct hisi_hba *)sas_ha->lldd_ha; lldd_ha is a void pointer, so you don't need a cast. > + port = &hisi_hba->port[i]; > + break; > + } > + i++; > + } The loop is really odd, as you apparently only try to find the array index to a pointer you already have. Is there no space for driver-specific data in 'asd_sas_phy'? If there is, just point to per-phy structure that you define yourself and put the index into that structure. I believe you already have a struct like that. > + if (hisi_hba == NULL) { When checking a pointer for validity, do not compare against NULL, but write this as 'if (!hisi_hba)', which is the more normal coding style. > + pr_err("%s: could not find hba\n", __func__); > + return; > + } Better use dev_err() to print the device name, but remove the __func__ argument. Again, when you have the per-phy structure, put a pointer to the device in there. > + > + if (lock) > + spin_lock_irqsave(&hisi_hba->lock, flags); > + port->port_attached = 1; > + port->id = phy->port_id; > + phy->port = port; > + sas_port->lldd_port = port; > + > + if (lock) > + spin_unlock_irqrestore(&hisi_hba->lock, flags); > +} This breaks some checking tools that try to validate the uses of locks. Better wrap the function in another one depending on the caller. When using spinlocks in general, it's also better to replace the "I have no clue where I'm called from" spin_lock_irqsave() call with either spin_lock() or spin_lock_irq() if at all possible. Arnd -- 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 20/25] hpsa: add discovery polling for PT RAID devices.
On 10/29/2015 03:59 PM, Matthew R. Ochs wrote: On Oct 29, 2015, at 3:51 PM, Don Brace wrote: On 10/29/2015 03:20 PM, Matthew R. Ochs wrote: On Oct 28, 2015, at 5:06 PM, Don Brace wrote: From: Scott Teel There are problems with getting configuration change notification in pass-through RAID environments. So, activate flag h->discovery_polling when one of these devices is detected in update_scsi_devices. After discovery_polling is set, execute a report luns from rescan_controller_worker (every 30 seconds). If the data from report_luns is different than last time (binary compare), execute a full rescan via update_scsi_devices. Reviewed-by: Scott Teel Reviewed-by: Justin Lindley Reviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 68 +++ drivers/scsi/hpsa.h |2 ++ 2 files changed, 70 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 8d67648..e521acd 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, static void hpsa_command_resubmit_worker(struct work_struct *work); static u32 lockup_detected(struct ctlr_info *h); static int detect_controller_lockup(struct ctlr_info *h); +static int hpsa_luns_changed(struct ctlr_info *h); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) { @@ -3904,6 +3905,18 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes); this_device = currentsd[ncurrent]; + /* Turn on discovery_polling if there are ext target devices. +* Event-based change notification is unreliable for those. +*/ + if (!h->discovery_polling) { + if (tmpdevice->external) { + h->discovery_polling = 1; + dev_info(&h->pdev->dev, + "External target, activate discovery polling.\n"); + } + } + + *this_device = *tmpdevice; this_device->physical_device = physical_device; @@ -8022,6 +8035,41 @@ static int hpsa_offline_devices_ready(struct ctlr_info *h) return 0; } +static int hpsa_luns_changed(struct ctlr_info *h) +{ + int rc = 1; /* assume there are changes */ + struct ReportLUNdata *logdev = NULL; + + /* if we can't find out if lun data has changed, +* assume that it has. +*/ + + if (!h->lastlogicals) + goto out; + + logdev = kzalloc(sizeof(*logdev), GFP_KERNEL); + if (!logdev) { + dev_warn(&h->pdev->dev, + "Out of memory, can't track lun changes.\n"); + goto out; + } + if (hpsa_scsi_do_report_luns(h, 1, logdev, sizeof(*logdev), 0)) { + dev_warn(&h->pdev->dev, + "report luns failed, can't track lun changes.\n"); + goto out; + } + if (memcmp(logdev, h->lastlogicals, sizeof(*logdev))) { + dev_info(&h->pdev->dev, + "Lun changes detected.\n"); + memcpy(h->lastlogicals, logdev, sizeof(*logdev)); + goto out; + } else + rc = 0; /* no changes detected. */ +out: + kfree(logdev); + return rc; +} + static void hpsa_rescan_ctlr_worker(struct work_struct *work) { unsigned long flags; @@ -8037,6 +8085,18 @@ static void hpsa_rescan_ctlr_worker(struct work_struct *work) hpsa_ack_ctlr_events(h); hpsa_scan_start(h->scsi_host); scsi_host_put(h->scsi_host); + } else if (h->discovery_polling) { + if (hpsa_luns_changed(h)) { + struct Scsi_Host *sh = NULL; + + dev_info(&h->pdev->dev, + "driver discovery polling rescan.\n"); + sh = scsi_host_get(h->scsi_host); + if (sh != NULL) { + hpsa_scan_start(sh); + scsi_host_put(sh); + } + } } spin_lock_irqsave(&h->lock, flags); if (!h->remove_in_progress) @@ -8277,6 +8337,8 @@ reinit_after_soft_reset: /* Enable Accelerated IO path at driver layer */ h->acciopath_status = 1; + /* Disable discovery polling.*/ + h->discovery_polling = 0; /* Turn the interrupts on so we can service requests */ @@ -8284,6 +8346,11 @@ reinit_after_soft_reset: hpsa_hba_inquiry(h); + h->lastlogicals = kzalloc(sizeof(*(h->lastlogicals)), GFP_KERNEL); + if (!h->lastlogicals) + dev_info(&h->pdev->dev, + "Can't track change to report lun data\n");
Re: [PATCH v2 27/32] scsi: hisi_sas: add smp protocol support
On Monday 26 October 2015 22:14:58 John Garry wrote: > + /* > + * DMA-map SMP request, response buffers > + */ > + /* req */ > + sg_req = &task->smp_task.smp_req; > + elem = dma_map_sg(dev, sg_req, 1, DMA_TO_DEVICE); > + if (!elem) > + return -ENOMEM; > + req_len = sg_dma_len(sg_req); > + req_dma_addr = sg_dma_address(sg_req); If you only use the first element, could you just use dma_map_single()? > + hdr->cmd_table_addr_lo = cpu_to_le32(lower_32_bits(req_dma_addr)); > + hdr->cmd_table_addr_hi = cpu_to_le32(upper_32_bits(req_dma_addr)); > + > + hdr->sts_buffer_addr_lo = > + cpu_to_le32(lower_32_bits(slot->status_buffer_dma)); > + hdr->sts_buffer_addr_hi = > + cpu_to_le32(upper_32_bits(slot->status_buffer_dma)); > + > I see these a lot in your code. Could you replace this wit hdr->cmd_table_addr = cpu_to_le64(req_dma_addr); and so on? That would be much more readable. Or are the two __le32 variables swapped? If so, you could add a small helper function like static inline __le64 cpu_to_le64_wordswapped(u64 val) { return cpu_to_le64(val >> 32 | val << 32); } Arnd -- 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: Blk-mq/scsi-mq Tuning
On 10/30/2015 02:25 PM, Chad Dupuis wrote: > > > On Fri, 30 Oct 2015, Hannes Reinecke wrote: > >> On 10/28/2015 09:11 PM, Chad Dupuis wrote: >>> Hi Folks, >>> >>> We¹ve begun to explore blk-mq and scsi-mq and wanted to know if there >>> were >>> any best practices in terms of block layer settings. We¹re looking >>> specifically at the FCoE and iSCSI protocols. >>> >>> A little background on the queues in our hardware first: we have a per >>> connection transmit queue and multiple, global receive queues. The >>> transmit queues are not pegged to a particular CPU. The receive queues >>> are pegged to the first N CPUs where N is the number of receive queues. >>> We set the nr_hw_queues in the scsi_host_template to N as well. >>> >> Weelll ... I think you'll run into issues here. >> The whole point of the multiqueue implementation is that you can tag the >> submission _and_ completion queue to a single CPU, thereby eliminating >> locking. >> If you only peg the completion queue to a CPU you'll still have >> contention on the submission queue, needing to take locks etc. >> >> Plus you will _inevitably_ incur cache misses, as the completion will >> basically never occur on the same CPU which did the submissoin. >> Hence the context needs to be bounced to the CPU holding the completion >> queue, or you'll need to do a IPI to inform the submitting CPU. >> But if you do that you're essentially doing single-queue submission, >> so I doubt we're seeing that great improvements. > > This was why I was asking if there was a blk-mq API to be able to set > CPU affinity for the hardware context queues so I could steer the > submissions to the CPUs that my receive queues are on (even if they are > allowed to float). > But what would that achieve? Each of the hardware context queues would still having to use the same submission queue, so you'd have to have some serialisation with spinlocks et.al. during submission. Which is what blk-mq tries to avoid. Am I wrong? 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
Re: Blk-mq/scsi-mq Tuning
On Fri, 30 Oct 2015, Hannes Reinecke wrote: On 10/28/2015 09:11 PM, Chad Dupuis wrote: Hi Folks, We¹ve begun to explore blk-mq and scsi-mq and wanted to know if there were any best practices in terms of block layer settings. We¹re looking specifically at the FCoE and iSCSI protocols. A little background on the queues in our hardware first: we have a per connection transmit queue and multiple, global receive queues. The transmit queues are not pegged to a particular CPU. The receive queues are pegged to the first N CPUs where N is the number of receive queues. We set the nr_hw_queues in the scsi_host_template to N as well. Weelll ... I think you'll run into issues here. The whole point of the multiqueue implementation is that you can tag the submission _and_ completion queue to a single CPU, thereby eliminating locking. If you only peg the completion queue to a CPU you'll still have contention on the submission queue, needing to take locks etc. Plus you will _inevitably_ incur cache misses, as the completion will basically never occur on the same CPU which did the submissoin. Hence the context needs to be bounced to the CPU holding the completion queue, or you'll need to do a IPI to inform the submitting CPU. But if you do that you're essentially doing single-queue submission, so I doubt we're seeing that great improvements. This was why I was asking if there was a blk-mq API to be able to set CPU affinity for the hardware context queues so I could steer the submissions to the CPUs that my receive queues are on (even if they are allowed to float). In our initial testing we¹re not seeing the performance scale as we would expect so we wanted to see if there some Œknobs¹ if you will that we could try tuning to try to increase the performance. Also, one question we did have is there an official API to be able to set the CPU affinity of the hw_ctx_queues? As above, given the underlying design I'm not surprised. But above you mentioned 'per-connection submission queues'; from which one could infer that there are several _hardware_ submission queues? If so, _maybe_ we should look into doing MC/S (in the iSCSI case), which would allow us to keep the 1:1 submission/completion ratio preferred by blk-mq and still use several queues ... Hmm? Yes, each connection has a transmit queue. Cheers, Hannes
Re: [PATCH v3 RESEND] scsi: report 'INQUIRY result too short' once per host
Hannes Reinecke writes: > On 10/30/2015 12:40 PM, Vitaly Kuznetsov wrote: >> Some host adapters (e.g. Hyper-V storvsc) are known for not respecting the >> SPC-2/3/4 requirement for 'INQUIRY data (see table ...) shall contain at >> least 36 bytes'. As a result we get tons on 'scsi 0:7:1:1: scsi scan: >> INQUIRY result too short (5), using 36' messages on console. This can be >> problematic for slow consoles. Introduce short_inquiry flag in struct >> Scsi_Host to print the message once per host. >> >> Signed-off-by: Vitaly Kuznetsov >> --- >> Changes since v3 RESEND: >> - Sorry, I screwed up James address when sending 'v3 RESEND'. >> >> Changes since v3: >> - no changes, this is just a RESEND. >> >> Changes since v2: >> - This is a successor of previously sent (and still not merged) "scsi: >> introduce short_inquiry flag for broken host adapters" patch. I'm not >> particularly sure which solution is better but I'm leaning towards this >> one as it doesn't require changes to adapter drivers. >> --- >> drivers/scsi/scsi_scan.c | 9 ++--- >> include/scsi/scsi_host.h | 3 +++ >> 2 files changed, 9 insertions(+), 3 deletions(-) >> > Why not printk_once? > Should achieve the same result, and we're saving yet another flag... > It seems it makes sense to print this warning per-host (as we can have many hosts) and not just once. It's hard to come up with a reasonable message for printk_once() -- "some of your SCSI devices are buggy" sounds to vague. Here we say "this particular scsi controller is buggy". -- Vitaly -- 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 v3] scsi: stex: Remove use of struct timeval
On Friday 30 October 2015 12:58:33 Hannes Reinecke wrote: > > @@ -1076,7 +1069,7 @@ static int stex_ss_handshake(struct st_hba *hba) > > h->req_cnt = cpu_to_le16(hba->rq_count+1); > > h->status_sz = cpu_to_le16(sizeof(struct status_msg)); > > h->status_cnt = cpu_to_le16(hba->sts_count+1); > > - stex_gettime(&h->hosttime); > > + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); > > h->partner_type = HMU_PARTNER_TYPE; > > h->extra_offset = h->extra_size = 0; > > scratch_size = (hba->sts_count+1)*sizeof(u32); > > > Just remove 'hosttime' altogether. It serves no purpose whatsoever. > > Are you sure? It is defined in a struct that is shared with the HBA as part of hba->dma_mem, so unless you have access to the firmware, it's hard to guess what it might be used for inside of the firmware. Arnd -- 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 00/15] Big fixes, retries, handle a race condition
On 10/28/2015 02:13 PM, Yaniv Gardi wrote: > Important: > This serie of 15 small patches should be pushed after the series of 8 patches > "Fix error message and present UFS variant probe" > > 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 (15): > 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: set REQUEST_SENSE command size to 18 bytes > 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 | 408 > -- > drivers/scsi/ufs/ufshcd.h | 4 + > 2 files changed, 327 insertions(+), 85 deletions(-) > Except for patch 07/15: Reviewed-by: Hannes Reinecke 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
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. 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
[PATCH RESEND] scsi: Export SCSI Inquiry data to sysfs
Export the RAW SCSI Inquiry to sysfs as binfile. This way the data can be used by userland without the need to have and ioctl or use the sg_inq tool. Here is an example of the provided data linux:~ # hexdump /sys/class/scsi_device/1\:0\:0\:0/device/inquiry 000 8005 3205 001f 4551 554d 2020 2020 010 4551 554d 4420 4456 522d 4d4f 2020 2020 020 2e32 2e33 024 Signed-off-by: Johannes Thumshirn Reviewed-by: Hannes Reinecke --- drivers/scsi/scsi_sysfs.c | 24 1 file changed, 24 insertions(+) diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index b89..fdcf0ab 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -773,6 +773,29 @@ static struct bin_attribute dev_attr_vpd_##_page = { \ sdev_vpd_pg_attr(pg83); sdev_vpd_pg_attr(pg80); +static ssize_t show_inquiry(struct file *filep, struct kobject *kobj, + struct bin_attribute *bin_attr, + char *buf, loff_t off, size_t count) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct scsi_device *sdev = to_scsi_device(dev); + + if (!sdev->inquiry) + return -EINVAL; + + return memory_read_from_buffer(buf, count, &off, sdev->inquiry, + sdev->inquiry_len); +} + +static struct bin_attribute dev_attr_inquiry = { + .attr = { + .name = "inquiry", + .mode = S_IRUGO, + }, + .size = 0, + .read = show_inquiry, +}; + static ssize_t show_iostat_counterbits(struct device *dev, struct device_attribute *attr, char *buf) @@ -957,6 +980,7 @@ static struct attribute *scsi_sdev_attrs[] = { static struct bin_attribute *scsi_sdev_bin_attrs[] = { &dev_attr_vpd_pg83, &dev_attr_vpd_pg80, + &dev_attr_inquiry, NULL }; static struct attribute_group scsi_sdev_attr_group = { -- 2.6.2 -- 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 v3] scsi: stex: Remove use of struct timeval
On 10/30/2015 09:30 AM, Tina Ruchandani wrote: > Function stex_gettime uses 'struct timeval' whose tv_sec value > will overflow on 32-bit systems in year 2038 and beyond. This patch > replaces the use of struct timeval and do_gettimeofday with > ktime_get_real_seconds, which returns a 64-bit seconds value. > > Suggested-by: Arnd Bergmann > Signed-off-by: Tina Ruchandani > -- > Changes in v3: > - Remove stex_gettime altogether, directly assign the timestamp. > Changes in v2: > - Change subject line to indicate that the patch is restricted to stex driver. > --- > --- > drivers/scsi/stex.c | 13 +++-- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c > index 98a62bc..84e196e 100644 > --- a/drivers/scsi/stex.c > +++ b/drivers/scsi/stex.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -362,14 +363,6 @@ MODULE_DESCRIPTION("Promise Technology SuperTrak EX > Controllers"); > MODULE_LICENSE("GPL"); > MODULE_VERSION(ST_DRIVER_VERSION); > > -static void stex_gettime(__le64 *time) > -{ > - struct timeval tv; > - > - do_gettimeofday(&tv); > - *time = cpu_to_le64(tv.tv_sec); > -} > - > static struct status_msg *stex_get_status(struct st_hba *hba) > { > struct status_msg *status = hba->status_buffer + hba->status_tail; > @@ -1002,7 +995,7 @@ static int stex_common_handshake(struct st_hba *hba) > h->req_cnt = cpu_to_le16(hba->rq_count+1); > h->status_sz = cpu_to_le16(sizeof(struct status_msg)); > h->status_cnt = cpu_to_le16(hba->sts_count+1); > - stex_gettime(&h->hosttime); > + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); > h->partner_type = HMU_PARTNER_TYPE; > if (hba->extra_offset) { > h->extra_offset = cpu_to_le32(hba->extra_offset); > @@ -1076,7 +1069,7 @@ static int stex_ss_handshake(struct st_hba *hba) > h->req_cnt = cpu_to_le16(hba->rq_count+1); > h->status_sz = cpu_to_le16(sizeof(struct status_msg)); > h->status_cnt = cpu_to_le16(hba->sts_count+1); > - stex_gettime(&h->hosttime); > + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); > h->partner_type = HMU_PARTNER_TYPE; > h->extra_offset = h->extra_size = 0; > scratch_size = (hba->sts_count+1)*sizeof(u32); > Just remove 'hosttime' altogether. It serves no purpose whatsoever. 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
Re: [PATCH v3 RESEND] scsi: report 'INQUIRY result too short' once per host
On 10/30/2015 12:40 PM, Vitaly Kuznetsov wrote: > Some host adapters (e.g. Hyper-V storvsc) are known for not respecting the > SPC-2/3/4 requirement for 'INQUIRY data (see table ...) shall contain at > least 36 bytes'. As a result we get tons on 'scsi 0:7:1:1: scsi scan: > INQUIRY result too short (5), using 36' messages on console. This can be > problematic for slow consoles. Introduce short_inquiry flag in struct > Scsi_Host to print the message once per host. > > Signed-off-by: Vitaly Kuznetsov > --- > Changes since v3 RESEND: > - Sorry, I screwed up James address when sending 'v3 RESEND'. > > Changes since v3: > - no changes, this is just a RESEND. > > Changes since v2: > - This is a successor of previously sent (and still not merged) "scsi: > introduce short_inquiry flag for broken host adapters" patch. I'm not > particularly sure which solution is better but I'm leaning towards this > one as it doesn't require changes to adapter drivers. > --- > drivers/scsi/scsi_scan.c | 9 ++--- > include/scsi/scsi_host.h | 3 +++ > 2 files changed, 9 insertions(+), 3 deletions(-) > Why not printk_once? Should achieve the same result, and we're saving yet another flag... 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
[PATCH v3 RESEND] scsi: report 'INQUIRY result too short' once per host
Some host adapters (e.g. Hyper-V storvsc) are known for not respecting the SPC-2/3/4 requirement for 'INQUIRY data (see table ...) shall contain at least 36 bytes'. As a result we get tons on 'scsi 0:7:1:1: scsi scan: INQUIRY result too short (5), using 36' messages on console. This can be problematic for slow consoles. Introduce short_inquiry flag in struct Scsi_Host to print the message once per host. Signed-off-by: Vitaly Kuznetsov --- Changes since v3 RESEND: - Sorry, I screwed up James address when sending 'v3 RESEND'. Changes since v3: - no changes, this is just a RESEND. Changes since v2: - This is a successor of previously sent (and still not merged) "scsi: introduce short_inquiry flag for broken host adapters" patch. I'm not particularly sure which solution is better but I'm leaning towards this one as it doesn't require changes to adapter drivers. --- drivers/scsi/scsi_scan.c | 9 ++--- include/scsi/scsi_host.h | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index f9f3f82..cd347e4 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -701,9 +701,12 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result, * strings. */ if (sdev->inquiry_len < 36) { - sdev_printk(KERN_INFO, sdev, - "scsi scan: INQUIRY result too short (%d)," - " using 36\n", sdev->inquiry_len); + if (!sdev->host->short_inquiry) { + shost_printk(KERN_INFO, sdev->host, + "scsi scan: INQUIRY result too short (%d)," + " using 36\n", sdev->inquiry_len); + sdev->host->short_inquiry = 1; + } sdev->inquiry_len = 36; } diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index e113c75..3a22da7 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -673,6 +673,9 @@ struct Scsi_Host { unsigned use_blk_mq:1; unsigned use_cmd_list:1; + /* Host responded with short (<36 bytes) INQUIRY result */ + unsigned short_inquiry:1; + /* * Optional work queue to be utilized by the transport */ -- 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
[PATCH RESEND] scsi_scan: don't dump trace when scsi_prep_async_scan() is called twice
The only user of scsi_prep_async_scan() is scsi_scan_host() and it handles the situation correctly. Move 'called twice' reporting to debug level as well. The issue is observed on Hyper-V: on any device add/remove event storvsc driver calls scsi_scan_host() and in case previous scan is still running we get the message and stack dump on console. Signed-off-by: Vitaly Kuznetsov Reviewed-by: K. Y. Srinivasan Tested-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- Changes since v1: - No changes, this is just a resend. --- drivers/scsi/scsi_scan.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index f9f3f82..01ad016 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1712,8 +1712,7 @@ static struct async_scan_data *scsi_prep_async_scan(struct Scsi_Host *shost) return NULL; if (shost->async_scan) { - shost_printk(KERN_INFO, shost, "%s called twice\n", __func__); - dump_stack(); + shost_printk(KERN_DEBUG, shost, "%s called twice\n", __func__); return NULL; } -- 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
[PATCH v3 RESEND] scsi: report 'INQUIRY result too short' once per host
Some host adapters (e.g. Hyper-V storvsc) are known for not respecting the SPC-2/3/4 requirement for 'INQUIRY data (see table ...) shall contain at least 36 bytes'. As a result we get tons on 'scsi 0:7:1:1: scsi scan: INQUIRY result too short (5), using 36' messages on console. This can be problematic for slow consoles. Introduce short_inquiry flag in struct Scsi_Host to print the message once per host. Signed-off-by: Vitaly Kuznetsov --- Changes since v3: - no changes, this is just a RESEND. Changes since v2: - This is a successor of previously sent (and still not merged) "scsi: introduce short_inquiry flag for broken host adapters" patch. I'm not particularly sure which solution is better but I'm leaning towards this one as it doesn't require changes to adapter drivers. --- drivers/scsi/scsi_scan.c | 9 ++--- include/scsi/scsi_host.h | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index f9f3f82..cd347e4 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -701,9 +701,12 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result, * strings. */ if (sdev->inquiry_len < 36) { - sdev_printk(KERN_INFO, sdev, - "scsi scan: INQUIRY result too short (%d)," - " using 36\n", sdev->inquiry_len); + if (!sdev->host->short_inquiry) { + shost_printk(KERN_INFO, sdev->host, + "scsi scan: INQUIRY result too short (%d)," + " using 36\n", sdev->inquiry_len); + sdev->host->short_inquiry = 1; + } sdev->inquiry_len = 36; } diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index e113c75..3a22da7 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -673,6 +673,9 @@ struct Scsi_Host { unsigned use_blk_mq:1; unsigned use_cmd_list:1; + /* Host responded with short (<36 bytes) INQUIRY result */ + unsigned short_inquiry:1; + /* * Optional work queue to be utilized by the transport */ -- 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: [RESEND PATCH] [SCSI] mvumi: 64bit value for seconds_since1970
On Fri, 2015-10-30 at 02:11 -0700, Tina Ruchandani wrote: > struct mvumi_hs_page2 stores a "seconds_since1970" field which is of > type u64. It is however, written to, using 'struct timeval' which has > a 32-bit seconds field and whose value will overflow in year 2038. > This patch uses ktime_get_real_seconds() instead since it provides a > 64-bit seconds value, which is 2038 safe. > > Signed-off-by: Tina Ruchandani Reviewed-by: Johannes Thumshirn > --- > drivers/scsi/mvumi.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c > index 3e6b866..02360de 100644 > --- a/drivers/scsi/mvumi.c > +++ b/drivers/scsi/mvumi.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -858,8 +859,8 @@ static void mvumi_hs_build_page(struct mvumi_hba > *mhba, > struct mvumi_hs_page2 *hs_page2; > struct mvumi_hs_page4 *hs_page4; > struct mvumi_hs_page3 *hs_page3; > - struct timeval time; > - unsigned int local_time; > + u64 time; > + u64 local_time; > > switch (hs_header->page_code) { > case HS_PAGE_HOST_INFO: > @@ -877,9 +878,8 @@ static void mvumi_hs_build_page(struct mvumi_hba > *mhba, > hs_page2->slot_number = 0; > hs_page2->intr_level = 0; > hs_page2->intr_vector = 0; > - do_gettimeofday(&time); > - local_time = (unsigned int) (time.tv_sec - > - (sys_tz.tz_minuteswe > st * 60)); > + time = ktime_get_real_seconds(); > + local_time = (time - (sys_tz.tz_minuteswest * 60)); > hs_page2->seconds_since1970 = local_time; > hs_header->checksum = > mvumi_calculate_checksum(hs_header, > hs_header- > >frame_length); -- 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] qla2xxx: Remove use of 'struct timeval'
On Fri, 2015-10-30 at 02:15 -0700, Tina Ruchandani wrote: > struct register_host_info stores a 64-bit UTC system time timestamp. > This patch removes the use of 'struct timeval' to obtain that > timestamp > as its tv_sec value will overflow on 32-bit systems in year 2038 > beyond. > The patch uses ktime_get_real_seconds() which returns a 64-bit > seconds value. > > Signed-off-by: Tina Ruchandani Reviewed-by: Johannes Thumshirn > --- > drivers/scsi/qla2xxx/qla_mr.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_mr.c > b/drivers/scsi/qla2xxx/qla_mr.c > index 6d190b4..d64a64a 100644 > --- a/drivers/scsi/qla2xxx/qla_mr.c > +++ b/drivers/scsi/qla2xxx/qla_mr.c > @@ -6,6 +6,7 @@ > */ > #include "qla_def.h" > #include > +#include > #include > #include > #include > @@ -1812,7 +1813,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t > *fcport, uint16_t fx_type) > struct host_system_info *phost_info; > struct register_host_info *preg_hsi; > struct new_utsname *p_sysid = NULL; > - struct timeval tv; > > sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL); > if (!sp) > @@ -1886,8 +1886,7 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t > *fcport, uint16_t fx_type) > p_sysid->domainname, DOMNAME_LENGTH); > strncpy(phost_info->hostdriver, > QLA2XXX_VERSION, VERSION_LENGTH); > - do_gettimeofday(&tv); > - preg_hsi->utc = (uint64_t)tv.tv_sec; > + preg_hsi->utc = > (uint64_t)ktime_get_real_seconds(); > ql_dbg(ql_dbg_init, vha, 0x0149, > "ISP%04X: Host registration with > firmware\n", > ha->pdev->device); -- 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] mpt2sas: Remove usage of 'struct timeval'
> Can you wait a cycle on this? Martin Petersen is on a mission to wrangle the > mpt2/3sas merger in this window, which will conflict with this. > James, No issues. Making a note here though that the patches for mpt2sas_base.c and mpt3sas_base.c are identical, I submitted the latter before I saw your email. Tina -- 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
[RESEND PATCH] qla2xxx: Remove use of 'struct timeval'
struct register_host_info stores a 64-bit UTC system time timestamp. This patch removes the use of 'struct timeval' to obtain that timestamp as its tv_sec value will overflow on 32-bit systems in year 2038 beyond. The patch uses ktime_get_real_seconds() which returns a 64-bit seconds value. Signed-off-by: Tina Ruchandani --- drivers/scsi/qla2xxx/qla_mr.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c index 6d190b4..d64a64a 100644 --- a/drivers/scsi/qla2xxx/qla_mr.c +++ b/drivers/scsi/qla2xxx/qla_mr.c @@ -6,6 +6,7 @@ */ #include "qla_def.h" #include +#include #include #include #include @@ -1812,7 +1813,6 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) struct host_system_info *phost_info; struct register_host_info *preg_hsi; struct new_utsname *p_sysid = NULL; - struct timeval tv; sp = qla2x00_get_sp(vha, fcport, GFP_KERNEL); if (!sp) @@ -1886,8 +1886,7 @@ qlafx00_fx_disc(scsi_qla_host_t *vha, fc_port_t *fcport, uint16_t fx_type) p_sysid->domainname, DOMNAME_LENGTH); strncpy(phost_info->hostdriver, QLA2XXX_VERSION, VERSION_LENGTH); - do_gettimeofday(&tv); - preg_hsi->utc = (uint64_t)tv.tv_sec; + preg_hsi->utc = (uint64_t)ktime_get_real_seconds(); ql_dbg(ql_dbg_init, vha, 0x0149, "ISP%04X: Host registration with firmware\n", ha->pdev->device); -- 2.2.0.rc0.207.ga3a616c -- 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] mpt2sas: Remove usage of 'struct timeval'
On October 30, 2015 6:05:15 PM GMT+09:00, Tina Ruchandani wrote: >'struct timeval' will have its tv_sec value overflow on 32-bit systems >in year 2038 and beyond. This patch replaces the use of struct timeval >for computing mpi_request.TimeStamp, and instead uses ktime_t which >provides >64-bit seconds value. The timestamp computed remains unaffected >(milliseconds >since Unix epoch). > >Signed-off-by: Tina Ruchandani Can you wait a cycle on this? Martin Petersen is on a mission to wrangle the mpt2/3sas merger in this window, which will conflict with this. James > drivers/scsi/mpt2sas/mpt2sas_base.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > >diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c >b/drivers/scsi/mpt2sas/mpt2sas_base.c >index 58e4521..6d80477 100644 >--- a/drivers/scsi/mpt2sas/mpt2sas_base.c >+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c >@@ -57,6 +57,7 @@ > #include > #include > #include >+#include > #include > #include > >@@ -3616,7 +3617,7 @@ _base_send_ioc_init(struct MPT2SAS_ADAPTER *ioc, >int sleep_flag) > Mpi2IOCInitRequest_t mpi_request; > Mpi2IOCInitReply_t mpi_reply; > int i, r = 0; >- struct timeval current_time; >+ ktime_t current_time; > u16 ioc_status; > u32 reply_post_free_array_sz = 0; > Mpi2IOCInitRDPQArrayEntry *reply_post_free_array = NULL; >@@ -3678,9 +3679,8 @@ _base_send_ioc_init(struct MPT2SAS_ADAPTER *ioc, >int sleep_flag) > /* This time stamp specifies number of milliseconds >* since epoch ~ midnight January 1, 1970. >*/ >- do_gettimeofday(¤t_time); >- mpi_request.TimeStamp = cpu_to_le64((u64)current_time.tv_sec * 1000 + >- (current_time.tv_usec / 1000)); >+ current_time = ktime_get_real(); >+ mpi_request.TimeStamp = cpu_to_le64(ktime_to_ms(current_time)); > > if (ioc->logging_level & MPT_DEBUG_INIT) { > __le32 *mfp; -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- 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
[RESEND PATCH] [SCSI] mvumi: 64bit value for seconds_since1970
struct mvumi_hs_page2 stores a "seconds_since1970" field which is of type u64. It is however, written to, using 'struct timeval' which has a 32-bit seconds field and whose value will overflow in year 2038. This patch uses ktime_get_real_seconds() instead since it provides a 64-bit seconds value, which is 2038 safe. Signed-off-by: Tina Ruchandani --- drivers/scsi/mvumi.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c index 3e6b866..02360de 100644 --- a/drivers/scsi/mvumi.c +++ b/drivers/scsi/mvumi.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -858,8 +859,8 @@ static void mvumi_hs_build_page(struct mvumi_hba *mhba, struct mvumi_hs_page2 *hs_page2; struct mvumi_hs_page4 *hs_page4; struct mvumi_hs_page3 *hs_page3; - struct timeval time; - unsigned int local_time; + u64 time; + u64 local_time; switch (hs_header->page_code) { case HS_PAGE_HOST_INFO: @@ -877,9 +878,8 @@ static void mvumi_hs_build_page(struct mvumi_hba *mhba, hs_page2->slot_number = 0; hs_page2->intr_level = 0; hs_page2->intr_vector = 0; - do_gettimeofday(&time); - local_time = (unsigned int) (time.tv_sec - - (sys_tz.tz_minuteswest * 60)); + time = ktime_get_real_seconds(); + local_time = (time - (sys_tz.tz_minuteswest * 60)); hs_page2->seconds_since1970 = local_time; hs_header->checksum = mvumi_calculate_checksum(hs_header, hs_header->frame_length); -- 2.2.0.rc0.207.ga3a616c -- 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
[RESEND PATCH] mpt3sas: Remove usage of 'struct timeval'
'struct timeval' will have its tv_sec value overflow on 32-bit systems in year 2038 and beyond. This patch replaces the use of struct timeval for computing mpi_request.TimeStamp, and instead uses ktime_t which provides 64-bit seconds value. The timestamp computed remains unaffected (milliseconds since Unix epoch). Signed-off-by: Tina Ruchandani --- drivers/scsi/mpt3sas/mpt3sas_base.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 1560115..ee9c3c6 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -56,6 +56,7 @@ #include #include #include +#include #include #include @@ -3735,7 +3736,7 @@ _base_send_ioc_init(struct MPT3SAS_ADAPTER *ioc, int sleep_flag) Mpi2IOCInitRequest_t mpi_request; Mpi2IOCInitReply_t mpi_reply; int i, r = 0; - struct timeval current_time; + ktime_t current_time; u16 ioc_status; u32 reply_post_free_array_sz = 0; Mpi2IOCInitRDPQArrayEntry *reply_post_free_array = NULL; @@ -3797,9 +3798,8 @@ _base_send_ioc_init(struct MPT3SAS_ADAPTER *ioc, int sleep_flag) /* This time stamp specifies number of milliseconds * since epoch ~ midnight January 1, 1970. */ - do_gettimeofday(¤t_time); - mpi_request.TimeStamp = cpu_to_le64((u64)current_time.tv_sec * 1000 + - (current_time.tv_usec / 1000)); + current_time = ktime_get_real(); + mpi_request.TimeStamp = cpu_to_le64(ktime_to_ms(current_time)); if (ioc->logging_level & MPT_DEBUG_INIT) { __le32 *mfp; -- 2.2.0.rc0.207.ga3a616c -- 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
[RESEND PATCH] mpt2sas: Remove usage of 'struct timeval'
'struct timeval' will have its tv_sec value overflow on 32-bit systems in year 2038 and beyond. This patch replaces the use of struct timeval for computing mpi_request.TimeStamp, and instead uses ktime_t which provides 64-bit seconds value. The timestamp computed remains unaffected (milliseconds since Unix epoch). Signed-off-by: Tina Ruchandani --- drivers/scsi/mpt2sas/mpt2sas_base.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index 58e4521..6d80477 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -57,6 +57,7 @@ #include #include #include +#include #include #include @@ -3616,7 +3617,7 @@ _base_send_ioc_init(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) Mpi2IOCInitRequest_t mpi_request; Mpi2IOCInitReply_t mpi_reply; int i, r = 0; - struct timeval current_time; + ktime_t current_time; u16 ioc_status; u32 reply_post_free_array_sz = 0; Mpi2IOCInitRDPQArrayEntry *reply_post_free_array = NULL; @@ -3678,9 +3679,8 @@ _base_send_ioc_init(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) /* This time stamp specifies number of milliseconds * since epoch ~ midnight January 1, 1970. */ - do_gettimeofday(¤t_time); - mpi_request.TimeStamp = cpu_to_le64((u64)current_time.tv_sec * 1000 + - (current_time.tv_usec / 1000)); + current_time = ktime_get_real(); + mpi_request.TimeStamp = cpu_to_le64(ktime_to_ms(current_time)); if (ioc->logging_level & MPT_DEBUG_INIT) { __le32 *mfp; -- 2.2.0.rc0.207.ga3a616c -- 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 v3] scsi: stex: Remove use of struct timeval
> > Thanks for the conversion. Can you please check if other (scsi) drivers > have the same y2038 issues? A quick "git grep do_gettimeofday > drivers/scsi/ | wc -l" reveals 30 occurrences (of cause not all are > problematic). > Hi Johannes, Yes, there are quite a few occurrences of timeval within scsi. I had sent some of the trivial back in the Feb-May 2015 period, and they were ack-ed by my then mentor and a couple of other people, but not merged or ack-ed by someone from linux-scsi. Until today, I thought using "RESEND" would be impolite, but now I will resend the other ones as well. Arnd Bergmann is leading this effort and may have more insightful comments. Thanks, Tina -- 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 v3] scsi: stex: Remove use of struct timeval
Hi, On Fri, 2015-10-30 at 01:30 -0700, Tina Ruchandani wrote: > Function stex_gettime uses 'struct timeval' whose tv_sec value > will overflow on 32-bit systems in year 2038 and beyond. This patch > replaces the use of struct timeval and do_gettimeofday with > ktime_get_real_seconds, which returns a 64-bit seconds value. Thanks for the conversion. Can you please check if other (scsi) drivers have the same y2038 issues? A quick "git grep do_gettimeofday drivers/scsi/ | wc -l" reveals 30 occurrences (of cause not all are problematic). Other than that Reviewed-by: Johannes Thumshirn -- 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 v9 1/8] phy: qcom-ufs: fix build error when the component is built as a module
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > Export the following functions in order to avoid build errors > when the component PHY_QCOM_UFS is compiled as a module: > > ERROR: "ufs_qcom_phy_disable_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_is_pcs_ready" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_disable_iface_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_start_serdes" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_calibrate_phy" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_dev_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_set_tx_lane_enable" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_disable_dev_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_save_controller_version" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_iface_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > make[1]: *** [__modpost] Error 1 > > Reviewed-by: Akinobu Mita > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > > --- > drivers/phy/phy-qcom-ufs.c | 11 +++ > 1 file changed, 11 insertions(+) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 2/8] scsi: ufs-qcom: fix compilation warning if compiled as a module
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > This change fixes a compilation warning that happens if SCSI_UFS_QCOM > is compiled as a module. > Also this patch fixes an error happens when insmod the module: > "ufs_qcom: module license 'unspecified' taints kernel." > > Reviewed-by: Akinobu Mita > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 5/8] scsi: ufs: creates wrapper functions for vops
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > In order to simplify the code a set of wrapper functions is created > to test and call each of the variant operations. > > Reviewed-by: Akinobu Mita > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 1 - > drivers/scsi/ufs/ufshcd.c | 104 > +--- > drivers/scsi/ufs/ufshcd.h | 98 + > 3 files changed, 137 insertions(+), 66 deletions(-) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 7/8] scsi: ufs-qcom: add debug prints for test bus
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > Adds support for configuring and reading the test bus and debug > registers. This change also adds another vops in order to print the > debug registers. > > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 8/8] scsi: ufs-qcom: add QUniPro hardware support and power optimizations
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > New revisions of UFS host controller supports the new UniPro > hardware controller (referred as QUniPro). This patch adds > the support to enable this new UniPro controller hardware. > > This change also adds power optimization for bus scaling feature, > as well as support for HS-G3 power mode. > > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > Reviewed-by: Hannes Reinecke 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
[RESEND PATCH v3] scsi: stex: Remove use of struct timeval
Function stex_gettime uses 'struct timeval' whose tv_sec value will overflow on 32-bit systems in year 2038 and beyond. This patch replaces the use of struct timeval and do_gettimeofday with ktime_get_real_seconds, which returns a 64-bit seconds value. Suggested-by: Arnd Bergmann Signed-off-by: Tina Ruchandani -- Changes in v3: - Remove stex_gettime altogether, directly assign the timestamp. Changes in v2: - Change subject line to indicate that the patch is restricted to stex driver. --- --- drivers/scsi/stex.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c index 98a62bc..84e196e 100644 --- a/drivers/scsi/stex.c +++ b/drivers/scsi/stex.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -362,14 +363,6 @@ MODULE_DESCRIPTION("Promise Technology SuperTrak EX Controllers"); MODULE_LICENSE("GPL"); MODULE_VERSION(ST_DRIVER_VERSION); -static void stex_gettime(__le64 *time) -{ - struct timeval tv; - - do_gettimeofday(&tv); - *time = cpu_to_le64(tv.tv_sec); -} - static struct status_msg *stex_get_status(struct st_hba *hba) { struct status_msg *status = hba->status_buffer + hba->status_tail; @@ -1002,7 +995,7 @@ static int stex_common_handshake(struct st_hba *hba) h->req_cnt = cpu_to_le16(hba->rq_count+1); h->status_sz = cpu_to_le16(sizeof(struct status_msg)); h->status_cnt = cpu_to_le16(hba->sts_count+1); - stex_gettime(&h->hosttime); + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); h->partner_type = HMU_PARTNER_TYPE; if (hba->extra_offset) { h->extra_offset = cpu_to_le32(hba->extra_offset); @@ -1076,7 +1069,7 @@ static int stex_ss_handshake(struct st_hba *hba) h->req_cnt = cpu_to_le16(hba->rq_count+1); h->status_sz = cpu_to_le16(sizeof(struct status_msg)); h->status_cnt = cpu_to_le16(hba->sts_count+1); - stex_gettime(&h->hosttime); + h->hosttime = cpu_to_le64(ktime_get_real_seconds()); h->partner_type = HMU_PARTNER_TYPE; h->extra_offset = h->extra_size = 0; scratch_size = (hba->sts_count+1)*sizeof(u32); -- 2.2.0.rc0.207.ga3a616c -- 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 v9 6/8] scsi: ufs: make the UFS variant a platform device
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > This change turns the UFS variant (SCSI_UFS_QCOM) into a UFS > a platform device. > In order to do so a few additional changes are required: > 1. The ufshcd-pltfrm is no longer serves as a platform device. >Now it only serves as a group of platform APIs such as PM APIs >(runtime suspend/resume, system suspend/resume etc), parsers of >clocks, regulators and pm_levels from DT. > 2. What used to be the old platform "probe" is now "only" >a pltfrm_init() routine, that does exactly the same, but only >being called by the new probe function of the UFS variant. > > Reviewed-by: Rob Herring > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 4/8] scsi: ufs: add ufshcd_get_variant ufshcd_set_variant
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > This patch adds ufshcd_get_variant() and ufshcd_set_variant() > routines in order to get/set the variant specific data. > > Reviewed-by: Akinobu Mita > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > Reviewed-by: Hannes Reinecke 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
Re: [PATCH v9 3/8] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component
On 10/28/2015 12:15 PM, Yaniv Gardi wrote: > This change is required in order to be able to build the component > as a module. > > Reviewed-by: Akinobu Mita > Reviewed-by: Subhash Jadavani > Reviewed-by: Gilad Broner > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig > index e945383..5f45307 100644 > --- a/drivers/scsi/ufs/Kconfig > +++ b/drivers/scsi/ufs/Kconfig > @@ -72,7 +72,7 @@ config SCSI_UFSHCD_PLATFORM > If unsure, say N. > > config SCSI_UFS_QCOM > - bool "QCOM specific hooks to UFS controller platform driver" > + tristate "QCOM specific hooks to UFS controller platform driver" > depends on SCSI_UFSHCD_PLATFORM && ARCH_QCOM > select PHY_QCOM_UFS > help > Reviewed-by: Hannes Reinecke 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
Re: [PATCH] scsi: restart list search after unlock in scsi_remove_target
Hi Bart, On Tue, 2015-10-27 at 13:14 -0700, Bart Van Assche wrote: > On 10/26/2015 01:35 AM, Johannes Thumshirn wrote: > > I haven't heard anything from the original reporter of the lockup > > but > > my test's went all O.K., so > > > > Tested-by: Johannes Thumshirn > > Reviewed-by: Johannes Thumshirn > > Hello Christoph and Johannes, > > How about the patch below, which is a variant of Christoph's patch ? > > Thanks, > > Bart. I'm OK with it and I ran tests with it over night, no lockups happened. I feel a bit reluctant of giving the patch to mt downsream reporter as it's not their business to validate patches for me. So from my POV it's OK as well. So please feel free to add Tested-by: Johannes Thumshirn Reviewed-by: Johannes Thumshirn on the official submission. Thanks, Johannes -- 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 25/25] hpsa: bump the driver version
On 10/28/2015 11:07 PM, Don Brace wrote: > Reviewed-by: Justin Lindley > Reviewed-by: Scott Teel > Reviewed-by: Kevin Barnett > Reviewed-by: Gerry Morong > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 24/25] hpsa: add in sas transport class
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Kevin Barnett > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 535 > +-- > drivers/scsi/hpsa.h | 27 ++ > drivers/scsi/hpsa_cmd.h | 14 + > 3 files changed, 555 insertions(+), 21 deletions(-) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 23/25] hpsa: fix multiple issues in path_info_show
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Rasmus Villemoes > > path_info_show() seems to be broken in multiple ways. > > First, there's > > 817 return snprintf(buf, output_len+1, "%s%s%s%s%s%s%s%s", > 818 path[0], path[1], path[2], path[3], > 819 path[4], path[5], path[6], path[7]); > > so hopefully output_len contains the combined length of the eight > strings. Otherwise, snprintf will stop copying to the output > buffer, but still end up reporting that combined length - which > in turn would result in user-space getting a bunch of useless nul > bytes (thankfully the upper sysfs layer seems to clear the output > buffer before passing it to the various ->show routines). But we have > > 767 output_len = snprintf(path[i], > 768 PATH_STRING_LEN, "[%d:%d:%d:%d] %20.20s ", > 769 h->scsi_host->host_no, > 770 hdev->bus, hdev->target, hdev->lun, > 771 scsi_device_type(hdev->devtype)); > > so output_len at best contains the length of the last string printed. > > Inside the loop, we then otherwise add to output_len. By magic, > we still have PATH_STRING_LEN available every time... This > wouldn't really be a problem if the bean-counting has been done > properly and each line actually does fit in 50 bytes, and maybe > it does, but I don't immediately see why. Suppose we end up > taking this branch: > > 802 output_len += snprintf(path[i] + output_len, > 803 PATH_STRING_LEN, > 804 "BOX: %hhu BAY: %hhu %s\n", > 805 box, bay, active); > > An optimistic estimate says this uses strlen("BOX: 1 BAY: 2 > Active\n") which is 21. Now add the 20 bytes guaranteed by the > %20.20s and then some for the rest of that format string, and > we're easily over 50 bytes. I don't think we can get over 100 > bytes even being pessimistic, so this just means we'll scribble > into the next path[i+1] and maybe get that overwritten later, > leading to some garbled output (in fact, since we'd overwrite the > previous string's 0-terminator, we could end up with one very > long string and then print various suffixes of that, leading to > much more than 400 bytes of output). Except of course when we're > filling path[7], where overrunning it means writing random stuff > to the kernel stack, which is usually a lot of fun. > > We can fix all of that and get rid of the 400 byte stack buffer by > simply writing directly to the given output buffer, which the upper > layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where > it is writing to, so this doesn't make the spin lock hold time any > longer. Using scnprintf ensures that output_len always represents the > number of bytes actually written to the buffer, so we'll report the > proper amount to the upper layer. > > Signed-off-by: Rasmus Villemoes > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 38 +- > 1 file changed, 17 insertions(+), 21 deletions(-) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 22/25] hpsa: enhance device messages
On 10/28/2015 11:06 PM, Don Brace wrote: > Reviewed-by: Justin Lindley > Reviewed-by: Scott Teel > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 45 - > 1 file changed, 40 insertions(+), 5 deletions(-) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 21/25] hpsa: disable report lun data caching
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Scott Teel > > When external target arrays are present, disable the firmware's > normal behavior of returning a cached copy of the report lun data, > and force it to collect new data each time we request a report luns. > > This is necessary for external arrays, since there may be no > reliable signal from the external array to the smart array when > lun configuration changes, and thus when driver requests > report luns, it may be stale data. > > Use diag options to turn off RPL data caching. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 86 > +++ > drivers/scsi/hpsa_cmd.h |3 ++ > 2 files changed, 89 insertions(+) > Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 20/25] hpsa: add discovery polling for PT RAID devices.
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Scott Teel > > There are problems with getting configuration change notification > in pass-through RAID environments. So, activate flag > h->discovery_polling when one of these devices is detected in > update_scsi_devices. > > After discovery_polling is set, execute a report luns from > rescan_controller_worker (every 30 seconds). > > If the data from report_luns is different than last > time (binary compare), execute a full rescan via update_scsi_devices. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 68 > +++ > drivers/scsi/hpsa.h |2 ++ > 2 files changed, 70 insertions(+) > Hmm. Can't say I like it, but if there's no other way... Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 19/25] hpsa: eliminate fake lun0 enclosures
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Scott Teel > > We don't need to create fake enclosure devices at Lun0 > in external target array configurations anymore. > This was done to support Pre-SCSI rev 5 controllers > that didn't suppoprt report luns commands, so the > SCSI layer had to scan targets. If there was no > LUN at LUN 0, then the target scan would stop, and > move to the next target. Lun0 enclosure device > was added to prevent sparsely-numbered LUNs from > being missed. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 18/25] External array LUNs must use target and lun numbers assigned by the
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Scott Teel > > external array. So the driver must treat these differently from > local LUNs when assigning lun/target. > Please fixup the description; looks like it has been incorrectly broken up during git commit. > LUN's 'model' field has been used to detect Lun types that need > special treatment, but the desire is to eliminate the need to reference > specific array models, and support any external array. > > Pass-through RAID (PTRAID) luns are not luns of the local controller, > so they are not reported in LUN count of command 'ID controller'. > However, they ARE reported in "Report logical Luns" command. > Local luns are listed first, then PTRAID LUNs. > > The number of luns from "Report LUNs" in excess of those reported by > 'ID controller' are therefore the PTRAID LUNS. > > We can now remove function is_ext_target, and the 'white list' > array of supported model names. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- Reviewed-by: Hannes Reinecke 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
Re: [PATCH 1 16/25] hpsa: refactor hpsa_figure_bus_target_lun
On 10/28/2015 11:06 PM, Don Brace wrote: > From: Kevin Barnett > > setup for sas transport. Need to set the > bus and target accordingly. > > Reviewed-by: Scott Teel > Reviewed-by: Justin Lindley > Reviewed-by: Kevin Barnett > Signed-off-by: Don Brace > --- > drivers/scsi/hpsa.c | 25 - > drivers/scsi/hpsa.h |5 + > 2 files changed, 17 insertions(+), 13 deletions(-) > Reviewed-by: Hannes Reinecke 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