[PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Sagi Grimberg]

If the backing device require stable pages, we need to set it
on the target as well. This applies to rdma/fc transports when
doing data integrity and tcp transport calculating digests.

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
This time it even compiles :)

 drivers/nvme/host/core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 222925dfe364..8b3827370874 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1907,6 +1907,13 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
 	if (ns->head->disk) {
 		nvme_update_disk_info(ns->head->disk, ns, id);
 		blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
+		if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
+			struct backing_dev_info *info =
+				ns->head->disk->queue->backing_dev_info;
+
+                        info->capabilities |= BDI_CAP_STABLE_WRITES;
+		}
+
 		revalidate_disk(ns->head->disk);
 	}
 #endif
-- 
2.20.1


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Christoph Hellwig]

On Tue, Mar 31, 2020 at 11:06:25PM -0700, Sagi Grimberg wrote:
> If the backing device require stable pages, we need to set it
> on the target as well. This applies to rdma/fc transports when
> doing data integrity and tcp transport calculating digests.

target?? Seems like this propagates it to the multipath node.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Sagi Grimberg]

>> If the backing device require stable pages, we need to set it
>> on the target as well. This applies to rdma/fc transports when
>> doing data integrity and tcp transport calculating digests.
> 
> target?? Seems like this propagates it to the multipath node.

typo..

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Christoph Hellwig]

On Tue, Mar 31, 2020 at 11:06:25PM -0700, Sagi Grimberg wrote:
> @@ -1907,6 +1907,13 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
>  	if (ns->head->disk) {
>  		nvme_update_disk_info(ns->head->disk, ns, id);
>  		blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
> +		if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
> +			struct backing_dev_info *info =
> +				ns->head->disk->queue->backing_dev_info;
> +
> +                        info->capabilities |= BDI_CAP_STABLE_WRITES;
> +		}

I think this needs to go into blk_queue_stack_limits instead, otherwise
we have the same problem with other stacking drivers.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Sagi Grimberg]

>> @@ -1907,6 +1907,13 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
>>   	if (ns->head->disk) {
>>   		nvme_update_disk_info(ns->head->disk, ns, id);
>>   		blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
>> +		if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
>> +			struct backing_dev_info *info =
>> +				ns->head->disk->queue->backing_dev_info;
>> +
>> +                        info->capabilities |= BDI_CAP_STABLE_WRITES;
>> +		}
> 
> I think this needs to go into blk_queue_stack_limits instead, otherwise
> we have the same problem with other stacking drivers.

I thought about this, but the stack_limits has different variants 
(blk_stack_limits, bdev_stack_limits) but only the first takes a
request_queue...

I see that dm-table does roughly the same thing, drbd ignores it.
In general, dm is doing a whole bunch of stacking limits/capabilities
related stuff that are not involved in blk_stack_limits...

I could theoretically add a flag to queue_limits to mirror this, is
that what you are suggesting?

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Sagi Grimberg]

If the backing device require stable pages, we need to set it on the
stack mpath device as well. This applies to rdma/fc transports when
doing data integrity and tcp transport calculating digests.

Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
Changes from v1:
- fixed change log description

 drivers/nvme/host/core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 222925dfe364..8b3827370874 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1907,6 +1907,13 @@ static void __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
 	if (ns->head->disk) {
 		nvme_update_disk_info(ns->head->disk, ns, id);
 		blk_queue_stack_limits(ns->head->disk->queue, ns->queue);
+		if (bdi_cap_stable_pages_required(ns->queue->backing_dev_info)) {
+			struct backing_dev_info *info =
+				ns->head->disk->queue->backing_dev_info;
+
+                        info->capabilities |= BDI_CAP_STABLE_WRITES;
+		}
+
 		revalidate_disk(ns->head->disk);
 	}
 #endif
-- 
2.20.1


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Christoph Hellwig]

On Wed, Apr 01, 2020 at 12:11:09PM -0700, Sagi Grimberg wrote:
>> I think this needs to go into blk_queue_stack_limits instead, otherwise
>> we have the same problem with other stacking drivers.
>
> I thought about this, but the stack_limits has different variants 
> (blk_stack_limits, bdev_stack_limits) but only the first takes a
> request_queue...
>
> I see that dm-table does roughly the same thing, drbd ignores it.
> In general, dm is doing a whole bunch of stacking limits/capabilities
> related stuff that are not involved in blk_stack_limits...
>
> I could theoretically add a flag to queue_limits to mirror this, is
> that what you are suggesting?

I guess we'll just go with your v4 patch for now and I'll see if I
can refactor this mess later..

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH v2] nvme: inherit stable pages constraint in the mpath stack device

[Christoph Hellwig]

Thanks,

applied to nvme-5.7.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme