* [PATCHv3] nvme: skip noiob for zoned devices
@ 2020-08-18 16:29 Keith Busch
2020-08-18 17:25 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2020-08-18 16:29 UTC (permalink / raw)
To: linux-nvme, sagi, hch; +Cc: Keith Busch
Zoned block devices reuse the chunk_sectors queue limit to define zone
boundaries. If a such a device happens to also report an optimal
boundary, log a warning of the conflicting attributes and do not use
that to define the chunk_sectors as that may intermittently interfere
with io splitting and zone size queries.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v2 -> v3:
Emit a warning on the initial validation if a zoned device reports an
optimal boundary.
drivers/nvme/host/core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 88cff309d8e4..10cf40d2cfef 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2075,8 +2075,13 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
}
}
- if (iob)
- blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
+ if (iob) {
+ if (!blk_queue_is_zoned(disk->queue))
+ blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
+ else if (!(disk->flags & GENHD_FL_UP))
+ pr_warn("%s: zone namespace has unused IO boundary:%u\n",
+ disk->disk_name, iob);
+ }
nvme_update_disk_info(disk, ns, id);
#ifdef CONFIG_NVME_MULTIPATH
if (ns->head->disk) {
--
2.24.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 16:29 [PATCHv3] nvme: skip noiob for zoned devices Keith Busch
@ 2020-08-18 17:25 ` Christoph Hellwig
2020-08-18 17:50 ` Keith Busch
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2020-08-18 17:25 UTC (permalink / raw)
To: Keith Busch; +Cc: sagi, linux-nvme, hch
On Tue, Aug 18, 2020 at 09:29:41AM -0700, Keith Busch wrote:
> Zoned block devices reuse the chunk_sectors queue limit to define zone
> boundaries. If a such a device happens to also report an optimal
> boundary, log a warning of the conflicting attributes and do not use
> that to define the chunk_sectors as that may intermittently interfere
> with io splitting and zone size queries.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v2 -> v3:
>
> Emit a warning on the initial validation if a zoned device reports an
> optimal boundary.
>
> drivers/nvme/host/core.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 88cff309d8e4..10cf40d2cfef 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2075,8 +2075,13 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
> }
> }
>
> - if (iob)
> - blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> + if (iob) {
> + if (!blk_queue_is_zoned(disk->queue))
> + blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> + else if (!(disk->flags & GENHD_FL_UP))
> + pr_warn("%s: zone namespace has unused IO boundary:%u\n",
> + disk->disk_name, iob);
This has some overly long line and looks fairly convoluted, what
about something like:
if (iob) {
unsigned int chunk_sectors = rounddown_pow_of_two(iob);
if (blk_queue_is_zoned(disk->queue) &&
chunk_sectors > blk_queue_zone_sectors(disk->queue))
pr_warn("%s: ignoring too large IO boundary: %u\n",
disk->disk_name, iob);
else
blk_queue_chunk_sectors(ns->queue, chunk_sectors);
}
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 17:25 ` Christoph Hellwig
@ 2020-08-18 17:50 ` Keith Busch
2020-08-18 18:02 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2020-08-18 17:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, linux-nvme
On Tue, Aug 18, 2020 at 07:25:04PM +0200, Christoph Hellwig wrote:
> > - if (iob)
> > - blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > + if (iob) {
> > + if (!blk_queue_is_zoned(disk->queue))
> > + blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > + else if (!(disk->flags & GENHD_FL_UP))
> > + pr_warn("%s: zone namespace has unused IO boundary:%u\n",
> > + disk->disk_name, iob);
>
> This has some overly long line and looks fairly convoluted, what
> about something like:
>
> if (iob) {
> unsigned int chunk_sectors = rounddown_pow_of_two(iob);
Unrelated to this particular patch, but ... why is this rounddown here?
While we can't use chunk_sectors if it is not a power of two, but what
benefit are we expecting to get from using the wrong boundary just
because it's convenient to our block stack?
> if (blk_queue_is_zoned(disk->queue) &&
> chunk_sectors > blk_queue_zone_sectors(disk->queue))
I don't think we should skip iob just for the case where chunk_sectors
is the larger value. A smaller value still ruins things like
blk_queue_zone_sectors() and blk_queue_zone_no().
Also, we call blk_revalidate_disk_zones() after the iob check, so the
blk_queue_zone_sectors() hasn't been initialized for this revalidate.
> pr_warn("%s: ignoring too large IO boundary: %u\n",
> disk->disk_name, iob);
We don't want to emit a repeated warning on every disk revalidation, so
I added the check for GENHD_FL_UP to warn only on the very first disk
validation.
> else
> blk_queue_chunk_sectors(ns->queue, chunk_sectors);
> }
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 17:50 ` Keith Busch
@ 2020-08-18 18:02 ` Christoph Hellwig
2020-08-18 19:10 ` Sagi Grimberg
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2020-08-18 18:02 UTC (permalink / raw)
To: Keith Busch; +Cc: Christoph Hellwig, linux-nvme, sagi
On Tue, Aug 18, 2020 at 10:50:54AM -0700, Keith Busch wrote:
> On Tue, Aug 18, 2020 at 07:25:04PM +0200, Christoph Hellwig wrote:
> > > - if (iob)
> > > - blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > > + if (iob) {
> > > + if (!blk_queue_is_zoned(disk->queue))
> > > + blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> > > + else if (!(disk->flags & GENHD_FL_UP))
> > > + pr_warn("%s: zone namespace has unused IO boundary:%u\n",
> > > + disk->disk_name, iob);
> >
> > This has some overly long line and looks fairly convoluted, what
> > about something like:
> >
> > if (iob) {
> > unsigned int chunk_sectors = rounddown_pow_of_two(iob);
>
> Unrelated to this particular patch, but ... why is this rounddown here?
> While we can't use chunk_sectors if it is not a power of two, but what
> benefit are we expecting to get from using the wrong boundary just
> because it's convenient to our block stack?
True, might make sense to add a validity check instead.
>
> > if (blk_queue_is_zoned(disk->queue) &&
> > chunk_sectors > blk_queue_zone_sectors(disk->queue))
>
> I don't think we should skip iob just for the case where chunk_sectors
> is the larger value. A smaller value still ruins things like
> blk_queue_zone_sectors() and blk_queue_zone_no().
>
> Also, we call blk_revalidate_disk_zones() after the iob check, so the
> blk_queue_zone_sectors() hasn't been initialized for this revalidate.
>
> > pr_warn("%s: ignoring too large IO boundary: %u\n",
> > disk->disk_name, iob);
>
> We don't want to emit a repeated warning on every disk revalidation, so
> I added the check for GENHD_FL_UP to warn only on the very first disk
> validation.
Ok. Maybe we just need to move the whole thing into a helper to look
less confusing.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 18:02 ` Christoph Hellwig
@ 2020-08-18 19:10 ` Sagi Grimberg
2020-08-18 19:20 ` Christoph Hellwig
0 siblings, 1 reply; 7+ messages in thread
From: Sagi Grimberg @ 2020-08-18 19:10 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch; +Cc: linux-nvme
I'll wait for a new version, reverting the existing patch
from nvme-5.9-rc for now...
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 19:10 ` Sagi Grimberg
@ 2020-08-18 19:20 ` Christoph Hellwig
2020-08-18 19:32 ` Sagi Grimberg
0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2020-08-18 19:20 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Keith Busch, Christoph Hellwig, linux-nvme
On Tue, Aug 18, 2020 at 12:10:22PM -0700, Sagi Grimberg wrote:
> I'll wait for a new version, reverting the existing patch
> from nvme-5.9-rc for now...
Let's just go back to v2, sorry for all the fuzz I caused.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHv3] nvme: skip noiob for zoned devices
2020-08-18 19:20 ` Christoph Hellwig
@ 2020-08-18 19:32 ` Sagi Grimberg
0 siblings, 0 replies; 7+ messages in thread
From: Sagi Grimberg @ 2020-08-18 19:32 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Keith Busch, linux-nvme
On 8/18/20 12:20 PM, Christoph Hellwig wrote:
> On Tue, Aug 18, 2020 at 12:10:22PM -0700, Sagi Grimberg wrote:
>> I'll wait for a new version, reverting the existing patch
>> from nvme-5.9-rc for now...
>
> Let's just go back to v2, sorry for all the fuzz I caused.
OK, requeued v2
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-18 19:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 16:29 [PATCHv3] nvme: skip noiob for zoned devices Keith Busch
2020-08-18 17:25 ` Christoph Hellwig
2020-08-18 17:50 ` Keith Busch
2020-08-18 18:02 ` Christoph Hellwig
2020-08-18 19:10 ` Sagi Grimberg
2020-08-18 19:20 ` Christoph Hellwig
2020-08-18 19:32 ` Sagi Grimberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.