Re: [PATCH 23/23] block: remove now unused queue limits helpers
On 3/25/24 08:54, Christoph Hellwig wrote: > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/bc4293a0-988d-42f3-a94a-b6715d72c204%40kernel.org.
Re: [PATCH 22/23] uas: switch to using ->device_configure to configure queue limits
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_alloc > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Note that uas was the only driver setting these size limits from > ->slave_alloc and not ->slave_configure and this makes it match > everyone else. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/43670f3c-b1cd-4732-9e0f-3cfb3ae9233a%40kernel.org.
Re: [PATCH 21/23] mpi3mr: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Note that mpi3mr also updates the limits from an event handler that > iterates all SCSI devices. This is also updated to use the > queue_limits, but the complete locking of this path probably means > it already is completely broken and needs a proper audit. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/66169fb4-efeb-4da5-8280-a0585537acfd%40kernel.org.
Re: [PATCH 20/23] libata: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. For the ata bits: Acked-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/06562b24-1397-4ac3-bb62-f2409503e956%40kernel.org.
Re: [PATCH 19/23] pata_macio: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Acked-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/6944da16-2a6c-42cd-baa1-c6d6d4dfc866%40kernel.org.
Re: [PATCH 18/23] sata_nv: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Acked-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/b1dba4fa-5a5d-443c-aae3-e8e5aea3eafe%40kernel.org.
Re: [PATCH 17/23] usb-storage: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Also use the proper atomic queue limit update helpers and freeze the > queue when updating max_hw_sectors from sysfs. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/9f2082fd-83d8-4140-84a2-865112090a46%40kernel.org.
Re: [PATCH 16/23] pmcraid: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/2f66396e-fe27-4a0b-a3fc-872bb6f07eee%40kernel.org.
Re: [PATCH 15/23] ipr: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/1890fdc9-41e4-415c-b210-f549e0b8436a%40kernel.org.
Re: [PATCH 14/23] hptiop: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/4f9b30f2-2351-4d58-b368-46c65288aef9%40kernel.org.
Re: [PATCH 13/23] sbp2: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/794533b4-19d7-4682-ad5f-9b19568420c3%40kernel.org.
Re: [PATCH 12/23] mpt3sas: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/b55b31ef-05ca-4487-ae7d-6d107c84f76c%40kernel.org.
Re: [PATCH 11/23] megaraid_sas: switch to using ->device_configure
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch to the ->device_configure method instead of ->slave_configure > and update the block limits on the passed in queue_limits instead > of using the per-limit accessors. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/004fec87-af1f-4a6a-aaa7-f57406bead41%40kernel.org.
Re: [PATCH 10/23] scsi: add a device_configure method to the host template
On 3/25/24 08:54, Christoph Hellwig wrote: > This is a version of ->slave_configure that also takes a queue_limits > structure that the caller applies, and thus allows drivers to reconfigure > the queue using the atomic queue limits API. > > In the long run it should also replace ->slave_configure entirely as > there is no need to have two different methods here, and the slave > name in addition to being politically charged also has no basis in > the SCSI standards or the kernel code. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/scsi_scan.c | 33 +++-- > include/scsi/scsi_host.h | 4 > 2 files changed, 23 insertions(+), 14 deletions(-) > > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c > index 699356d7d17545..8e05780f802523 100644 > --- a/drivers/scsi/scsi_scan.c > +++ b/drivers/scsi/scsi_scan.c > @@ -227,7 +227,7 @@ static int scsi_realloc_sdev_budget_map(struct > scsi_device *sdev, > > /* >* realloc if new shift is calculated, which is caused by setting > - * up one new default queue depth after calling ->slave_configure > + * up one new default queue depth after calling ->device_configure >*/ > if (!need_alloc && new_shift != sdev->budget_map.shift) > need_alloc = need_free = true; > @@ -874,8 +874,9 @@ static int scsi_probe_lun(struct scsi_device *sdev, > unsigned char *inq_result, > static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result, > blist_flags_t *bflags, int async) > { > + const struct scsi_host_template *hostt = sdev->host->hostt; > struct queue_limits lim; > - int ret; > + int ret, ret2; > > /* >* XXX do not save the inquiry, since it can change underneath us, > @@ -1073,22 +1074,26 @@ static int scsi_add_lun(struct scsi_device *sdev, > unsigned char *inq_result, > lim.max_hw_sectors = 512; > else if (*bflags & BLIST_MAX_1024) > lim.max_hw_sectors = 1024; > - ret = queue_limits_commit_update(sdev->request_queue, ); > + > + if (hostt->device_configure) > + ret = hostt->device_configure(sdev, ); > + else if (hostt->slave_configure) > + ret = hostt->slave_configure(sdev); > + > + ret2 = queue_limits_commit_update(sdev->request_queue, ); Why do this if ->device_configure() or ->slave_configure() failed ? Shouldn't the "if (ret) goto fail" hunk be moved above this call ? > diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h > index b0948ab69e0fa6..1959193d47e7f5 100644 > --- a/include/scsi/scsi_host.h > +++ b/include/scsi/scsi_host.h > @@ -211,7 +211,11 @@ struct scsi_host_template { >* up after yourself before returning non-0 >* >* Status: OPTIONAL > + * > + * Note: slave_configure is the legacy version, use device_configure for > + * all new code. Maybe explictly mention that both *cannot* be defined here ? > */ > + int (* device_configure)(struct scsi_device *, struct queue_limits > *lim); > int (* slave_configure)(struct scsi_device *); > > /* With these 2 nits addressed, looks all goo to me. Feel free to add: Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/6199c70e-f0a9-4756-b3fb-106985c41ebf%40kernel.org.
Re: [PATCH 09/23] scsi: use the atomic queue limits API in scsi_add_lun
On 3/25/24 08:54, Christoph Hellwig wrote: > Switch scsi_add_lun to use the atomic queue limits API to update the > max_hw_sectors for devices with quirks. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/942dc890-9a1d-4008-944d-816f7a7c470b%40kernel.org.
Re: [PATCH 08/23] ufs-exynos: move setting the the dma alignment to the init method
On 3/25/24 08:54, Christoph Hellwig wrote: > Use the SCSI host's dma_alignment field and set it in ->init and remove > the now unused config_scsi_dev method. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/0fa3b0d9-d6a8-4427-80a3-616e54987a77%40kernel.org.
Re: [PATCH 07/23] scsi: add a dma_alignment field to the host and host template
On 3/25/24 08:54, Christoph Hellwig wrote: > Get drivers out of the business of having to call the block layer > dma alignment limits helpers themselves. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/3f140e6e-a73b-4c27-a14f-0add8c36dd26%40kernel.org.
Re: [PATCH 06/23] scsi: add a no_highmem flag to struct Scsi_Host
On 3/25/24 08:54, Christoph Hellwig wrote: > While we really should be killing the block layer bounce buffering ASAP, > I even more urgently need to stop the drivers to fiddle with the limits > from ->slave_configure. Add a no_highmem flag to the Scsi_Host to > centralize this setting and switch the remaining four drivers that use > block layer bounce buffering to it. > > Signed-off-by: Christoph Hellwig The USB hunks could probably be moved to their own patch following this one ? But otherwise, looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/80162a6e-12d1-4fd4-ac74-dc5388853323%40kernel.org.
Re: [PATCH 05/23] scsi_transport_fc: add a max_bsg_segments field to struct fc_function_template
On 3/25/24 08:54, Christoph Hellwig wrote: > ibmvfc only supports a single segment for BSG FC passthrough. Instead of > having it set a queue limits after creating the BSD queues, add a field so > that the FC transport can set it before allocating the queue. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/08451976-7e39-4c7f-8bf5-5eeda4316c4b%40kernel.org.
Re: [PATCH 04/23] scsi: initialize scsi midlayer limits before allocating the queue
On 3/25/24 08:54, Christoph Hellwig wrote: > Turn __scsi_init_queue into scsi_init_limits which initializes > queue_limits structure that can be passed to blk_mq_alloc_queue. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/be1655f7-3ae0-4a5a-ac35-95e9c7d2da02%40kernel.org.
Re: [PATCH 03/23] mpi3mr: pass queue_limits to bsg_setup_queue
On 3/25/24 08:54, Christoph Hellwig wrote: > Pass the limits to bsg_setup_queue instead of setting them up on the live > queue. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/04beb70f-38b0-46f7-bbbc-24cf40a91d70%40kernel.org.
Re: [PATCH 02/23] bsg: pass queue_limits to bsg_setup_queue
On 3/25/24 08:54, Christoph Hellwig wrote: > This allows bsg_setup_queue to pass them to blk_mq_alloc_queue and thus > set up the limits at queue allocation time. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/c9070d06-3095-4bc9-8b9e-ce292404362c%40kernel.org.
Re: [PATCH 01/23] block: don't reject too large max_user_setors in blk_validate_limits
On 3/25/24 08:54, Christoph Hellwig wrote: > We already cap down the actual max_sectors to the max of the hardware > and user limit, so don't reject the configuration. > > Signed-off-by: Christoph Hellwig Looks OK to me. Reviewed-by: Damien Le Moal -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/b8346696-316c-497d-972e-c76d897a1c87%40kernel.org.
Re: [PATCH 1/2] scsi: core: cleanup scsi_dev_queue_ready()
On 2023/09/22 2:38, Wenchao Hao wrote: > This is just a cleanup for scsi_dev_queue_ready() to avoid > redundant goto and if statement, it did not change the origin > logic. > > Signed-off-by: Wenchao Hao > --- > drivers/scsi/scsi_lib.c | 35 ++- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index ca5eb058d5c7..f3e388127dbd 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -1254,28 +1254,29 @@ static inline int scsi_dev_queue_ready(struct > request_queue *q, > int token; > > token = sbitmap_get(>budget_map); > - if (atomic_read(>device_blocked)) { > - if (token < 0) > - goto out; > + if (token < 0) > + return -1; This is changing how this function works... > > - if (scsi_device_busy(sdev) > 1) > - goto out_dec; > + /* > + * device_blocked is not set at mostly time, so check it first > + * and return token when it is not set. > + */ > + if (!atomic_read(>device_blocked)) > + return token; ...because you reversed the tests order. > > - /* > - * unblock after device_blocked iterates to zero > - */ > - if (atomic_dec_return(>device_blocked) > 0) > - goto out_dec; > - SCSI_LOG_MLQUEUE(3, sdev_printk(KERN_INFO, sdev, > -"unblocking device at zero depth\n")); > + /* > + * unblock after device_blocked iterates to zero > + */ > + if (scsi_device_busy(sdev) > 1 || > + atomic_dec_return(>device_blocked) > 0) { And here too, you are changing how the function works. The atomic_dec may not be done if the first condition is true. > + sbitmap_put(>budget_map, token); > + return -1; > } > > + SCSI_LOG_MLQUEUE(3, sdev_printk(KERN_INFO, sdev, > + "unblocking device at zero depth\n")); > + > return token; > -out_dec: > - if (token >= 0) > - sbitmap_put(>budget_map, token); > -out: > - return -1; > } > > /* -- Damien Le Moal Western Digital Research -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/ea28de69-8b9d-8ff8-b7fc-eb780123f055%40kernel.org.