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.
