* [PATCH] nvme: make sure ns head inherits underlying device limits
@ 2018-10-19 22:24 Sagi Grimberg
2018-10-22 8:57 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2018-10-19 22:24 UTC (permalink / raw)
Whenever we update ns_head info, we need to make sure it is still
compatible with all underlying backing devices. Start with unlimited
stacking limits, and every info update iterate over siblings and
adjust queue limits.
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
drivers/nvme/host/core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index bdfbbeb4b2f8..6194b3ee30ca 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1480,6 +1480,7 @@ static bool nvme_ns_ids_equal(struct nvme_ns_ids *a, struct nvme_ns_ids *b)
static void nvme_update_disk_info(struct gendisk *disk,
struct nvme_ns *ns, struct nvme_id_ns *id)
{
+ struct nvme_ns *n;
sector_t capacity = le64_to_cpup(&id->nsze) << (ns->lba_shift - 9);
unsigned short bs = 1 << ns->lba_shift;
@@ -1504,6 +1505,9 @@ static void nvme_update_disk_info(struct gendisk *disk,
else
set_disk_ro(disk, false);
+ list_for_each_entry_rcu(n, &ns->head->list, siblings)
+ blk_queue_stack_limits(disk->queue, n->queue);
+
blk_mq_unfreeze_queue(disk->queue);
}
@@ -2958,6 +2962,8 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
if (ret)
goto out_cleanup_srcu;
+ blk_set_stacking_limits(&head->disk->queue->limits);
+
list_add_tail(&head->entry, &ctrl->subsys->nsheads);
kref_get(&ctrl->subsys->ref);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] nvme: make sure ns head inherits underlying device limits
2018-10-19 22:24 [PATCH] nvme: make sure ns head inherits underlying device limits Sagi Grimberg
@ 2018-10-22 8:57 ` Christoph Hellwig
2018-10-23 0:22 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2018-10-22 8:57 UTC (permalink / raw)
On Fri, Oct 19, 2018@03:24:50PM -0700, Sagi Grimberg wrote:
> Whenever we update ns_head info, we need to make sure it is still
> compatible with all underlying backing devices. Start with unlimited
> stacking limits, and every info update iterate over siblings and
> adjust queue limits.
Why? Neither the block layer, nor the nvme multipath code should
even look at this values.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: make sure ns head inherits underlying device limits
2018-10-22 8:57 ` Christoph Hellwig
@ 2018-10-23 0:22 ` Sagi Grimberg
2018-10-26 7:53 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2018-10-23 0:22 UTC (permalink / raw)
>> Whenever we update ns_head info, we need to make sure it is still
>> compatible with all underlying backing devices. Start with unlimited
>> stacking limits, and every info update iterate over siblings and
>> adjust queue limits.
>
> Why? Neither the block layer, nor the nvme multipath code should
> even look at this values.
Without this patch at least max_segments and max_hw_sectors of the head
device node are degenerated to defaults that have nothing to do with the
underlying device limits.
This can result in unnecessary bio splitting for the head device
node...
Although I did not check, same holds for discard sectors/segments.
Maybe this is an overkill? but thought this would be a better idea than
simply adjusting limits one-by-one depending on what I find...
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: make sure ns head inherits underlying device limits
2018-10-23 0:22 ` Sagi Grimberg
@ 2018-10-26 7:53 ` Christoph Hellwig
2018-10-30 18:11 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2018-10-26 7:53 UTC (permalink / raw)
On Mon, Oct 22, 2018@05:22:15PM -0700, Sagi Grimberg wrote:
> Without this patch at least max_segments and max_hw_sectors of the head
> device node are degenerated to defaults that have nothing to do with the
> underlying device limits.
>
> This can result in unnecessary bio splitting for the head device
> node...
But we never split bios for the multipath queue!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: make sure ns head inherits underlying device limits
2018-10-26 7:53 ` Christoph Hellwig
@ 2018-10-30 18:11 ` Sagi Grimberg
2018-10-31 5:27 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2018-10-30 18:11 UTC (permalink / raw)
Christoph,
>> Without this patch at least max_segments and max_hw_sectors of the head
>> device node are degenerated to defaults that have nothing to do with the
>> underlying device limits.
>>
>> This can result in unnecessary bio splitting for the head device
>> node...
>
> But we never split bios for the multipath queue!
Yes of course we don't, but we can still stack other devices on top
of the multipath device that can split bios and in fact rely on the
underlying queue limits.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] nvme: make sure ns head inherits underlying device limits
2018-10-30 18:11 ` Sagi Grimberg
@ 2018-10-31 5:27 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2018-10-31 5:27 UTC (permalink / raw)
On Tue, Oct 30, 2018@11:11:11AM -0700, Sagi Grimberg wrote:
> Christoph,
>
>>> Without this patch at least max_segments and max_hw_sectors of the head
>>> device node are degenerated to defaults that have nothing to do with the
>>> underlying device limits.
>>>
>>> This can result in unnecessary bio splitting for the head device
>>> node...
>>
>> But we never split bios for the multipath queue!
>
> Yes of course we don't, but we can still stack other devices on top
> of the multipath device that can split bios and in fact rely on the
> underlying queue limits.
Ok, that makes sense. Can you resubmit it with that in a comment?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-31 5:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-19 22:24 [PATCH] nvme: make sure ns head inherits underlying device limits Sagi Grimberg
2018-10-22 8:57 ` Christoph Hellwig
2018-10-23 0:22 ` Sagi Grimberg
2018-10-26 7:53 ` Christoph Hellwig
2018-10-30 18:11 ` Sagi Grimberg
2018-10-31 5:27 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).