| Message ID | 20200430131904.5847-1-hare@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | scsi: enable reserved commands for LLDDs | expand |
On 30/04/2020 14:18, Hannes Reinecke wrote: Thanks for this. > Conversion of the SAS drivers hisi_sas, pm8001, and mv_sas are > compile tested only; I'd be grateful if someone could give > these patches a spin on that hardware, too. So after some build fixups, I get this a NULL pointer deref: [ 5.565899] sas_find_dev_by_rphy+0x3c/0x104 [ 5.570182] sas_target_alloc+0x18/0x84 [ 5.574029] scsi_alloc_target+0x20c/0x304 [ 5.578136] scsi_get_virtual_dev+0x44/0xec [ 5.582331] sas_register_ha+0xd0/0x258 [ 5.586178] hisi_sas_probe+0x2ec/0x36c [ 5.590024] hisi_sas_v2_probe+0x34/0x64 [ 5.593958] platform_drv_probe+0x4c/0xa0 [ 5.597978] really_probe+0xd8/0x334 [ 5.601561] driver_probe_device+0x58/0xe8 [ 5.605669] device_driver_attach+0x68/0x70 [ 5.609864] __driver_attach+0x9c/0xf8 [ 5.613622] bus_for_each_dev+0x50/0xa0 [ 5.617468] driver_attach+0x20/0x28 [ 5.621051] bus_add_driver+0x148/0x1fc [ 5.624897] driver_register+0x6c/0x124 [ 5.628742] __platform_driver_register+0x48/0x50 [ 5.633463] hisi_sas_v2_driver_init+0x18/0x20 [ 5.637921] do_one_initcall+0x50/0x194 [ 5.641767] kernel_init_freeable+0x1e4/0x24c And so we need this change: commit 51f607bf91853026af102367d9e6666605cdec61 (HEAD) Author: John Garry <john.garry@huawei.com> Date: Fri May 1 12:30:32 2020 +0100 scsi: libsas: Don't attempt to find scsi host rphy in target alloc It doesn't have one. Signed-off-by: John Garry <john.garry@huawei.com> diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index 585e0df5fce2..f1a823d51044 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -822,8 +825,15 @@ struct domain_device *sas_find_dev_by_rphy(struct sas_rphy *rphy) int sas_target_alloc(struct scsi_target *starget) { - struct sas_rphy *rphy = dev_to_rphy(starget->dev.parent); - struct domain_device *found_dev = sas_find_dev_by_rphy(rphy); + struct device *parent = starget->dev.parent; + struct sas_rphy *rphy; + struct domain_device *found_dev; + + if (scsi_is_host_device(parent)) + return 0; + + rphy = dev_to_rphy(parent); + found_dev = sas_find_dev_by_rphy(rphy); Cheers,
Can we get the basic infrastructure sorted out with just say csiostor and virtio-scsi before we get into all the more complicated bits? A 40+ series gets close to impossible to review unless it is just all mechnical changes.
On 5/1/20 2:01 PM, John Garry wrote: > On 30/04/2020 14:18, Hannes Reinecke wrote: > > Thanks for this. > >> Conversion of the SAS drivers hisi_sas, pm8001, and mv_sas are >> compile tested only; I'd be grateful if someone could give >> these patches a spin on that hardware, too. > > So after some build fixups, I get this a NULL pointer deref: > > [ 5.565899] sas_find_dev_by_rphy+0x3c/0x104 > [ 5.570182] sas_target_alloc+0x18/0x84 > [ 5.574029] scsi_alloc_target+0x20c/0x304 > [ 5.578136] scsi_get_virtual_dev+0x44/0xec > [ 5.582331] sas_register_ha+0xd0/0x258 > [ 5.586178] hisi_sas_probe+0x2ec/0x36c > [ 5.590024] hisi_sas_v2_probe+0x34/0x64 > [ 5.593958] platform_drv_probe+0x4c/0xa0 > [ 5.597978] really_probe+0xd8/0x334 > [ 5.601561] driver_probe_device+0x58/0xe8 > [ 5.605669] device_driver_attach+0x68/0x70 > [ 5.609864] __driver_attach+0x9c/0xf8 > [ 5.613622] bus_for_each_dev+0x50/0xa0 > [ 5.617468] driver_attach+0x20/0x28 > [ 5.621051] bus_add_driver+0x148/0x1fc > [ 5.624897] driver_register+0x6c/0x124 > [ 5.628742] __platform_driver_register+0x48/0x50 > [ 5.633463] hisi_sas_v2_driver_init+0x18/0x20 > [ 5.637921] do_one_initcall+0x50/0x194 > [ 5.641767] kernel_init_freeable+0x1e4/0x24c > > And so we need this change: > > commit 51f607bf91853026af102367d9e6666605cdec61 (HEAD) > Author: John Garry <john.garry@huawei.com> > Date: Fri May 1 12:30:32 2020 +0100 > > scsi: libsas: Don't attempt to find scsi host rphy in target alloc > > It doesn't have one. > > Signed-off-by: John Garry <john.garry@huawei.com> > > diff --git a/drivers/scsi/libsas/sas_scsi_host.c > b/drivers/scsi/libsas/sas_scsi_host.c > index 585e0df5fce2..f1a823d51044 100644 > --- a/drivers/scsi/libsas/sas_scsi_host.c > +++ b/drivers/scsi/libsas/sas_scsi_host.c > @@ -822,8 +825,15 @@ struct domain_device *sas_find_dev_by_rphy(struct > sas_rphy *rphy) > > int sas_target_alloc(struct scsi_target *starget) > { > - struct sas_rphy *rphy = dev_to_rphy(starget->dev.parent); > - struct domain_device *found_dev = sas_find_dev_by_rphy(rphy); > + struct device *parent = starget->dev.parent; > + struct sas_rphy *rphy; > + struct domain_device *found_dev; > + > + if (scsi_is_host_device(parent)) > + return 0; > + > + rphy = dev_to_rphy(parent); > + found_dev = sas_find_dev_by_rphy(rphy); > > Thank you, will be including it in the next round. Cheers, Hannes
On 5/1/20 7:46 PM, Christoph Hellwig wrote: > Can we get the basic infrastructure sorted out with just say csiostor > and virtio-scsi before we get into all the more complicated bits? > A 40+ series gets close to impossible to review unless it is just > all mechnical changes. > Sure. This patchset was just an RFC to show where I would want to go. I'll be restricting it to the virtio and csiostor for the next round. Cheers, Hannes
git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git reserved-tags.v3 As usual, comments and reviews are welcome. I cloned your tree and ran some tests using hpsa. I used 3 SATA HBA disks, 3 SAS HBA disks, 2 LOGICAL VOLUMES using HDDs, and 2 LOGICAL VOLUMES using SDDs for ioaccel path. I have an IO stress test that does mkfs, mount/umount, fio, and fsck to the above volumes while doing sg_reset opeations in parallel. The stress test has survived 2 full days of testing. However, my insmod/rmmod test causes a kernel panic. Not sure why yet. I had re-cloned yesterday an d the panic is still there. Earlier kernel versions do not panic on rmmod. -- 2.16.4
On 6/5/20 5:32 PM, Don.Brace@microchip.com wrote: > > git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git reserved-tags.v3 > > As usual, comments and reviews are welcome. > > I cloned your tree and ran some tests using hpsa. > > I used 3 SATA HBA disks, 3 SAS HBA disks, 2 LOGICAL VOLUMES using HDDs, and 2 LOGICAL VOLUMES using SDDs for ioaccel path. > > I have an IO stress test that does mkfs, mount/umount, fio, and fsck to the above volumes while doing sg_reset opeations in parallel. > > The stress test has survived 2 full days of testing. > > However, my insmod/rmmod test causes a kernel panic. > > Not sure why yet. I had re-cloned yesterday an d the panic is still there. > > Earlier kernel versions do not panic on rmmod. > > -- > 2.16.4 > Ah, right. Of course. Should be fixed with this patch:\ diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index fdc291c47b9b..87469908524c 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1885,9 +1885,9 @@ void scsi_forget_host(struct Scsi_Host *shost) goto restart; } /* Remove virtual device last, might be needed to send commands */ + spin_unlock_irqrestore(shost->host_lock, flags); if (virtual_sdev) __scsi_remove_device(virtual_sdev); - spin_unlock_irqrestore(shost->host_lock, flags); } /** Can you test with that? Cheers, Hannes
Subject: Re: [PATCH RFC v3 00/41] scsi: enable reserved commands for LLDDs EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > However, my insmod/rmmod test causes a kernel panic. > > Ah, right. Of course. Should be fixed with this patch:\ Can you test with that? Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer Yes, I'll start today.
Subject: Re: [PATCH RFC v3 00/41] scsi: enable reserved commands for LLDDs On 6/5/20 5:32 PM, Don.Brace@microchip.com wrote: > > git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git > reserved-tags.v3 > > As usual, comments and reviews are welcome. > > I cloned your tree and ran some tests using hpsa. > > I used 3 SATA HBA disks, 3 SAS HBA disks, 2 LOGICAL VOLUMES using HDDs, and 2 LOGICAL VOLUMES using SDDs for ioaccel path. > > I have an IO stress test that does mkfs, mount/umount, fio, and fsck to the above volumes while doing sg_reset opeations in parallel. > > The stress test has survived 2 full days of testing. > > However, my insmod/rmmod test causes a kernel panic. > > Not sure why yet. I had re-cloned yesterday an d the panic is still there. > > Earlier kernel versions do not panic on rmmod. > > -- > 2.16.4 > >>Ah, right. Of course. >>Should be fixed with this patch:\ >>Can you test with that? --- Applied patch... Ran 8000 iterations of my io stress script with 10K reset operations in parallel over 3 days. insmod/rmmod hpsa_ioctl testing controller FW flashing kdump/kexec testing reboot Tested-by: Don Brace <don.brace@microsemi.com> Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
Hi all, quite some drivers use internal commands for various purposes, most commonly sending TMFs or querying the HBA status. While these commands use the same submission mechanism than normal I/O commands, they will not be counted as outstanding commands, requiring those drivers to implement their own mechanism to figure out outstanding commands. The block layer already has the concept of 'reserved' tags for precisely this purpose, namely non-I/O tags which live off a separate tag pool. That guarantees that these commands can always be sent, and won't be influenced by tag starvation from the I/O tag pool. This patchset enables the use of reserved tags for the SCSI midlayer by allocating a virtual LUN for the HBA itself which just serves as a resource to allocate valid tags from. This removes quite some hacks which were required for some drivers (eg. fnic or snic). Conversion of the SAS drivers hisi_sas, pm8001, and mv_sas are compile tested only; I'd be grateful if someone could give these patches a spin on that hardware, too. The entire patchset can be found at git://git.kernel.org/pub/scm/linux/kernel/git/hare/scsi-devel.git reserved-tags.v3 As usual, comments and reviews are welcome. Changes to v2: - Update patches from John Garry - Use virtual LUN as suggested by Christoph - Improve SCSI Host device to present a real SCSI device - Implement 'persistent' commands for AENs - Convert Megaraid SAS Changes to v1: - Make scsi_{get, put}_reserved_cmd() for Scsi host - Previously we separate scsi_{get, put}_reserved_cmd() for sdev and scsi_host_get_reserved_cmd() for the host - Fix how Scsi_Host.can_queue is set in the virtio-scsi change - Drop Scsi_Host.use_reserved_cmd_q - Drop scsi_is_reserved_cmd() - Add support in libsas and associated HBA drivers - Allocate reserved command in slow task - Switch hisi_sas to use reserved Scsi command - Reorder the series a little - Some tidying Hannes Reinecke (39): scsi: add 'nr_reserved_cmds' field to the SCSI host template scsi: add scsi_{get,put}_reserved_cmd() scsi: Implement scsi_cmd_is_reserved() csiostor: use reserved command for LUN reset scsi: add scsi_cmd_from_priv() virtio_scsi: use reserved commands for TMF fnic: use reserved commands fnic: use scsi_host_busy_iter() to traverse commands scsi: use real inquiry data when initialising devices scsi: make host device a first-class citizen hpsa: move hpsa_hba_inquiry after scsi_add_host() hpsa: use reserved commands hpsa: use scsi_host_busy_iter() to traverse outstanding commands hpsa: drop refcount field from CommandList aacraid: use private commands aacraid: use scsi_host_busy_iter() to traverse commands megaraid_sas: kill this_id and init_id megaraid_sas: use shost_priv() megaraid_sas: avoid using megaraid_lookup_instance() megaraid_sas: separate out megasas_set_max_sectors() megaraid_sas: megaraid_sas: reshuffle SCSI host allocation block: implement persistent commands scsi: add a 'persistent' argument to scsi_get_reserved_cmd() megaraid_sas: separate out megasas_prepare_aen() megaraid_sas: use reserved commands megaraid_sas_fusion: rearrange mfi and mpt frame pools megaraid_sas_fusion: sanitize command lookup megaraid_sas: use scsi_host_busy_iter to traverse outstanding commands snic: use reserved commands snic: use tagset iter for traversing commands mv_sas: kill mvsas_debug_issue_ssp_tmf() pm8001: kill pm8001_issue_ssp_tmf() pm8001: kill 'dev' argument from pm8001_exec_internal_task_abort() pm8001: use libsas-provided domain devices for SATA libsas: add SCSI target pointer to struct domain_device libsas: add tag to struct sas_task hisi_sas: use task tag to reference the slot mv_sas: use reserved tags and drop private tag allocation pm8001: use block-layer tags for ccb allocation John Garry (2): scsi: libsas,hisi_sas,mvsas,pm8001: Allocate Scsi_cmd for slow task scsi: hisi_sas: Use libsas slow task SCSI command block/blk-mq.c | 27 +- drivers/scsi/aacraid/aachba.c | 14 +- drivers/scsi/aacraid/aacraid.h | 9 +- drivers/scsi/aacraid/commctrl.c | 57 +- drivers/scsi/aacraid/comminit.c | 34 +- drivers/scsi/aacraid/commsup.c | 76 +-- drivers/scsi/aacraid/dpcsup.c | 2 +- drivers/scsi/aacraid/linit.c | 181 +++--- drivers/scsi/csiostor/csio_init.c | 1 + drivers/scsi/csiostor/csio_scsi.c | 48 +- drivers/scsi/fnic/fnic_scsi.c | 943 ++++++++++++---------------- drivers/scsi/hisi_sas/hisi_sas_main.c | 116 ++-- drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 5 +- drivers/scsi/hpsa.c | 367 +++++------ drivers/scsi/hpsa.h | 3 +- drivers/scsi/hpsa_cmd.h | 1 - drivers/scsi/libsas/sas_ata.c | 4 + drivers/scsi/libsas/sas_expander.c | 7 +- drivers/scsi/libsas/sas_init.c | 63 +- drivers/scsi/libsas/sas_scsi_host.c | 4 +- drivers/scsi/megaraid/megaraid_sas.h | 14 +- drivers/scsi/megaraid/megaraid_sas_base.c | 664 +++++++++++--------- drivers/scsi/megaraid/megaraid_sas_fusion.c | 431 +++++++------ drivers/scsi/megaraid/megaraid_sas_fusion.h | 5 - drivers/scsi/mvsas/mv_init.c | 10 +- drivers/scsi/mvsas/mv_sas.c | 136 ++-- drivers/scsi/mvsas/mv_sas.h | 13 +- drivers/scsi/pm8001/pm8001_hwi.c | 133 ++-- drivers/scsi/pm8001/pm8001_init.c | 30 +- drivers/scsi/pm8001/pm8001_sas.c | 192 +++--- drivers/scsi/pm8001/pm8001_sas.h | 13 +- drivers/scsi/pm8001/pm80xx_hwi.c | 131 ++-- drivers/scsi/scsi_lib.c | 73 +++ drivers/scsi/scsi_scan.c | 80 ++- drivers/scsi/scsi_sysfs.c | 3 +- drivers/scsi/snic/snic.h | 4 +- drivers/scsi/snic/snic_main.c | 8 + drivers/scsi/snic/snic_scsi.c | 386 ++++++------ drivers/scsi/virtio_scsi.c | 105 ++-- include/linux/blk-mq.h | 3 + include/linux/blk_types.h | 4 + include/scsi/libsas.h | 11 +- include/scsi/scsi_cmnd.h | 23 + include/scsi/scsi_device.h | 4 + include/scsi/scsi_host.h | 9 + 45 files changed, 2190 insertions(+), 2257 deletions(-)