* [PATCHv2] nvme: skip noiob for zoned devices
@ 2020-08-07 16:32 Keith Busch
2020-08-07 16:33 ` Sagi Grimberg
2020-08-10 12:33 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: Keith Busch @ 2020-08-07 16:32 UTC (permalink / raw)
To: linux-nvme, hch, sagi; +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, 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>
---
v1->v2: Fixed the if condition to check for *not* zoned.
drivers/nvme/host/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 88cff309d8e4..4e0225d85e70 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2075,7 +2075,7 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
}
}
- if (iob)
+ if (iob && !blk_queue_is_zoned(ns->queue))
blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
nvme_update_disk_info(disk, ns, id);
#ifdef CONFIG_NVME_MULTIPATH
--
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] 6+ messages in thread
* Re: [PATCHv2] nvme: skip noiob for zoned devices
2020-08-07 16:32 [PATCHv2] nvme: skip noiob for zoned devices Keith Busch
@ 2020-08-07 16:33 ` Sagi Grimberg
2020-08-10 12:33 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-08-07 16:33 UTC (permalink / raw)
To: Keith Busch, linux-nvme, hch
Queued for nvme-5.9-rc
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] nvme: skip noiob for zoned devices
2020-08-07 16:32 [PATCHv2] nvme: skip noiob for zoned devices Keith Busch
2020-08-07 16:33 ` Sagi Grimberg
@ 2020-08-10 12:33 ` Christoph Hellwig
2020-08-10 14:56 ` Keith Busch
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-08-10 12:33 UTC (permalink / raw)
To: Keith Busch; +Cc: hch, linux-nvme, sagi
On Fri, Aug 07, 2020 at 09:32:35AM -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, do not use that to define the chunk_sectors as that may
> intermittently interfere with io splitting and zone size queries.
Instead of skipping shouldn't we use the max of noiob and the zone
size, even if that is mostly theoretical?
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v1->v2: Fixed the if condition to check for *not* zoned.
>
> drivers/nvme/host/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 88cff309d8e4..4e0225d85e70 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2075,7 +2075,7 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
> }
> }
>
> - if (iob)
> + if (iob && !blk_queue_is_zoned(ns->queue))
> blk_queue_chunk_sectors(ns->queue, rounddown_pow_of_two(iob));
> nvme_update_disk_info(disk, ns, id);
> #ifdef CONFIG_NVME_MULTIPATH
> --
> 2.24.1
---end quoted text---
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] nvme: skip noiob for zoned devices
2020-08-10 12:33 ` Christoph Hellwig
@ 2020-08-10 14:56 ` Keith Busch
2020-08-14 6:28 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Keith Busch @ 2020-08-10 14:56 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, linux-nvme
On Mon, Aug 10, 2020 at 02:33:13PM +0200, Christoph Hellwig wrote:
> On Fri, Aug 07, 2020 at 09:32:35AM -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, do not use that to define the chunk_sectors as that may
> > intermittently interfere with io splitting and zone size queries.
>
> Instead of skipping shouldn't we use the max of noiob and the zone
> size, even if that is mostly theoretical?
No, I don't think so. If noiob is larger, we'll return the wrong value
for blk_queue_zone_sectors() and blkdev_nr_zones(), which will have
undesirable consequences.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] nvme: skip noiob for zoned devices
2020-08-10 14:56 ` Keith Busch
@ 2020-08-14 6:28 ` Christoph Hellwig
2020-08-14 6:34 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-08-14 6:28 UTC (permalink / raw)
To: Keith Busch; +Cc: Christoph Hellwig, linux-nvme, sagi
On Mon, Aug 10, 2020 at 07:56:22AM -0700, Keith Busch wrote:
> On Mon, Aug 10, 2020 at 02:33:13PM +0200, Christoph Hellwig wrote:
> > On Fri, Aug 07, 2020 at 09:32:35AM -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, do not use that to define the chunk_sectors as that may
> > > intermittently interfere with io splitting and zone size queries.
> >
> > Instead of skipping shouldn't we use the max of noiob and the zone
> > size, even if that is mostly theoretical?
>
> No, I don't think so. If noiob is larger, we'll return the wrong value
> for blk_queue_zone_sectors() and blkdev_nr_zones(), which will have
> undesirable consequences.
Then we at least need to warn about a larger noiob. And add a
comment that we are ignoring it deliberately.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] nvme: skip noiob for zoned devices
2020-08-14 6:28 ` Christoph Hellwig
@ 2020-08-14 6:34 ` Sagi Grimberg
0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-08-14 6:34 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch; +Cc: linux-nvme
>>> On Fri, Aug 07, 2020 at 09:32:35AM -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, do not use that to define the chunk_sectors as that may
>>>> intermittently interfere with io splitting and zone size queries.
>>>
>>> Instead of skipping shouldn't we use the max of noiob and the zone
>>> size, even if that is mostly theoretical?
>>
>> No, I don't think so. If noiob is larger, we'll return the wrong value
>> for blk_queue_zone_sectors() and blkdev_nr_zones(), which will have
>> undesirable consequences.
>
> Then we at least need to warn about a larger noiob. And add a
> comment that we are ignoring it deliberately.
I'm fine with that, Keith if you agree you can send a patch
and I'll fold it into the original one.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-14 6:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-07 16:32 [PATCHv2] nvme: skip noiob for zoned devices Keith Busch
2020-08-07 16:33 ` Sagi Grimberg
2020-08-10 12:33 ` Christoph Hellwig
2020-08-10 14:56 ` Keith Busch
2020-08-14 6:28 ` Christoph Hellwig
2020-08-14 6:34 ` Sagi Grimberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox