All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] nvmet: fix file & block device backed ns
@ 2019-03-26  3:19 Ming Lei
  2019-03-26  3:19   ` Ming Lei
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)


Hi,

The 1st patch sets loop queue's segment boundary mask as PAGE_SIZE - 1
because nvme target only accepts single-page sg list.

The 2nd & 3rd patches fix computation io vector.

The last patch builds bvec from sg directly.

Ming Lei (4):
  nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  nvmet: fix computation of io vector number
  nvmet: fix computation of bvec->bv_len
  nvmet: build bvec from sg directly

 drivers/nvme/target/io-cmd-file.c | 20 ++++++++++----------
 drivers/nvme/target/loop.c        |  3 +++
 2 files changed, 13 insertions(+), 10 deletions(-)

Cc: Yi Zhang <yi.zhang at redhat.com>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

-- 
2.9.5

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-26  3:19 [PATCH 0/4] nvmet: fix file & block device backed ns Ming Lei
@ 2019-03-26  3:19   ` Ming Lei
  2019-03-26  3:19   ` Ming Lei
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)
  To: linux-nvme, Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable

NVMe target only accepts single-page sg list, either file or block
device backed target code follows this assumption.

However, loop target is one exception, given the sg list is from
the host queue directly.

This patch sets loop queue's segment boundary mask as PAGE_SIZE - 1
for folowoing NVMe target assumption.

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Fixes: 3a85a5de29ea ("nvme-loop: add a NVMe loopback host driver")
Cc: Yi Zhang <yi.zhang@redhat.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/nvme/target/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index b9f623ab01f3..7194f86b9dac 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -549,6 +549,9 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
 	if (ret)
 		goto out_cleanup_connect_q;
 
+	/* target only accepts single-page sg list */
+	blk_queue_segment_boundary(ctrl->ctrl.connect_q, PAGE_SIZE - 1);
+
 	return 0;
 
 out_cleanup_connect_q:
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-26  3:19   ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)


NVMe target only accepts single-page sg list, either file or block
device backed target code follows this assumption.

However, loop target is one exception, given the sg list is from
the host queue directly.

This patch sets loop queue's segment boundary mask as PAGE_SIZE - 1
for folowoing NVMe target assumption.

Reported-by: Yi Zhang <yi.zhang at redhat.com>
Fixes: 3a85a5de29ea ("nvme-loop: add a NVMe loopback host driver")
Cc: Yi Zhang <yi.zhang at redhat.com>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Cc: <stable at vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei at redhat.com>
---
 drivers/nvme/target/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index b9f623ab01f3..7194f86b9dac 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -549,6 +549,9 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
 	if (ret)
 		goto out_cleanup_connect_q;
 
+	/* target only accepts single-page sg list */
+	blk_queue_segment_boundary(ctrl->ctrl.connect_q, PAGE_SIZE - 1);
+
 	return 0;
 
 out_cleanup_connect_q:
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/4] nvmet: fix computation of io vector number
  2019-03-26  3:19 [PATCH 0/4] nvmet: fix file & block device backed ns Ming Lei
@ 2019-03-26  3:19   ` Ming Lei
  2019-03-26  3:19   ` Ming Lei
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)
  To: linux-nvme, Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable

Current file backed ns code uses request data length to compute
number of io vector.

This way is actually wrong, the warning in nvmet_file_execute_io()
may be triggered because the sg->offset isn't considered.

Given the sg list is only single-page for nvme target, simply use
the req->sg_cnt as number of io vector.

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Fixes: d5eff33ee6f8("nvmet: add simple file backed ns support")
Cc: Yi Zhang <yi.zhang@redhat.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/nvme/target/io-cmd-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 3e43212d3c1c..d67a43832cb1 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -128,7 +128,7 @@ static void nvmet_file_io_done(struct kiocb *iocb, long ret, long ret2)
 
 static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 {
-	ssize_t nr_bvec = DIV_ROUND_UP(req->data_len, PAGE_SIZE);
+	ssize_t nr_bvec = req->sg_cnt;
 	struct sg_page_iter sg_pg_iter;
 	unsigned long bv_cnt = 0;
 	bool is_sync = false;
@@ -225,7 +225,7 @@ static void nvmet_file_submit_buffered_io(struct nvmet_req *req)
 
 static void nvmet_file_execute_rw(struct nvmet_req *req)
 {
-	ssize_t nr_bvec = DIV_ROUND_UP(req->data_len, PAGE_SIZE);
+	ssize_t nr_bvec = req->sg_cnt;
 
 	if (!req->sg_cnt || !nr_bvec) {
 		nvmet_req_complete(req, 0);
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/4] nvmet: fix computation of io vector number
@ 2019-03-26  3:19   ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)


Current file backed ns code uses request data length to compute
number of io vector.

This way is actually wrong, the warning in nvmet_file_execute_io()
may be triggered because the sg->offset isn't considered.

Given the sg list is only single-page for nvme target, simply use
the req->sg_cnt as number of io vector.

Reported-by: Yi Zhang <yi.zhang at redhat.com>
Fixes: d5eff33ee6f8("nvmet: add simple file backed ns support")
Cc: Yi Zhang <yi.zhang at redhat.com>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Cc: <stable at vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei at redhat.com>
---
 drivers/nvme/target/io-cmd-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 3e43212d3c1c..d67a43832cb1 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -128,7 +128,7 @@ static void nvmet_file_io_done(struct kiocb *iocb, long ret, long ret2)
 
 static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 {
-	ssize_t nr_bvec = DIV_ROUND_UP(req->data_len, PAGE_SIZE);
+	ssize_t nr_bvec = req->sg_cnt;
 	struct sg_page_iter sg_pg_iter;
 	unsigned long bv_cnt = 0;
 	bool is_sync = false;
@@ -225,7 +225,7 @@ static void nvmet_file_submit_buffered_io(struct nvmet_req *req)
 
 static void nvmet_file_execute_rw(struct nvmet_req *req)
 {
-	ssize_t nr_bvec = DIV_ROUND_UP(req->data_len, PAGE_SIZE);
+	ssize_t nr_bvec = req->sg_cnt;
 
 	if (!req->sg_cnt || !nr_bvec) {
 		nvmet_req_complete(req, 0);
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/4] nvmet: fix computation of bvec->bv_len
  2019-03-26  3:19 [PATCH 0/4] nvmet: fix file & block device backed ns Ming Lei
@ 2019-03-26  3:19   ` Ming Lei
  2019-03-26  3:19   ` Ming Lei
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)
  To: linux-nvme, Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable

PAGE_SIZE - sg->offset may be bigger than sg->length, so we have
to cap it.

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Fixes: d5eff33ee6f8("nvmet: add simple file backed ns support")
Cc: Yi Zhang <yi.zhang@redhat.com>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/nvme/target/io-cmd-file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index d67a43832cb1..149e17e699e5 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -79,7 +79,8 @@ static void nvmet_file_init_bvec(struct bio_vec *bv, struct sg_page_iter *iter)
 {
 	bv->bv_page = sg_page_iter_page(iter);
 	bv->bv_offset = iter->sg->offset;
-	bv->bv_len = PAGE_SIZE - iter->sg->offset;
+	bv->bv_len = min_t(unsigned, PAGE_SIZE - iter->sg->offset,
+			   iter->sg->length);
 }
 
 static ssize_t nvmet_file_submit_bvec(struct nvmet_req *req, loff_t pos,
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/4] nvmet: fix computation of bvec->bv_len
@ 2019-03-26  3:19   ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)


PAGE_SIZE - sg->offset may be bigger than sg->length, so we have
to cap it.

Reported-by: Yi Zhang <yi.zhang at redhat.com>
Fixes: d5eff33ee6f8("nvmet: add simple file backed ns support")
Cc: Yi Zhang <yi.zhang at redhat.com>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Cc: <stable at vger.kernel.org>
Signed-off-by: Ming Lei <ming.lei at redhat.com>
---
 drivers/nvme/target/io-cmd-file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index d67a43832cb1..149e17e699e5 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -79,7 +79,8 @@ static void nvmet_file_init_bvec(struct bio_vec *bv, struct sg_page_iter *iter)
 {
 	bv->bv_page = sg_page_iter_page(iter);
 	bv->bv_offset = iter->sg->offset;
-	bv->bv_len = PAGE_SIZE - iter->sg->offset;
+	bv->bv_len = min_t(unsigned, PAGE_SIZE - iter->sg->offset,
+			   iter->sg->length);
 }
 
 static ssize_t nvmet_file_submit_bvec(struct nvmet_req *req, loff_t pos,
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/4] nvmet: build bvec from sg directly
  2019-03-26  3:19 [PATCH 0/4] nvmet: fix file & block device backed ns Ming Lei
                   ` (2 preceding siblings ...)
  2019-03-26  3:19   ` Ming Lei
@ 2019-03-26  3:19 ` Ming Lei
  3 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  3:19 UTC (permalink / raw)


NVMe target only accepts single-page sg list, so build bvec
from sg directly.

Cc: Yi Zhang <yi.zhang at redhat.com>
Cc: Sagi Grimberg <sagi at grimberg.me>
Cc: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
Signed-off-by: Ming Lei <ming.lei at redhat.com>
---
 drivers/nvme/target/io-cmd-file.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 149e17e699e5..bc6ebb51b0bf 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -75,12 +75,11 @@ int nvmet_file_ns_enable(struct nvmet_ns *ns)
 	return ret;
 }
 
-static void nvmet_file_init_bvec(struct bio_vec *bv, struct sg_page_iter *iter)
+static void nvmet_file_init_bvec(struct bio_vec *bv, struct scatterlist *sg)
 {
-	bv->bv_page = sg_page_iter_page(iter);
-	bv->bv_offset = iter->sg->offset;
-	bv->bv_len = min_t(unsigned, PAGE_SIZE - iter->sg->offset,
-			   iter->sg->length);
+	bv->bv_page = sg_page(sg);
+	bv->bv_offset = sg->offset;
+	bv->bv_len = sg->length;
 }
 
 static ssize_t nvmet_file_submit_bvec(struct nvmet_req *req, loff_t pos,
@@ -130,13 +129,13 @@ static void nvmet_file_io_done(struct kiocb *iocb, long ret, long ret2)
 static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 {
 	ssize_t nr_bvec = req->sg_cnt;
-	struct sg_page_iter sg_pg_iter;
 	unsigned long bv_cnt = 0;
 	bool is_sync = false;
 	size_t len = 0, total_len = 0;
 	ssize_t ret = 0;
 	loff_t pos;
-
+	int i;
+	struct scatterlist *sg;
 
 	if (req->f.mpool_alloc && nr_bvec > NVMET_MAX_MPOOL_BVEC)
 		is_sync = true;
@@ -148,8 +147,8 @@ static bool nvmet_file_execute_io(struct nvmet_req *req, int ki_flags)
 	}
 
 	memset(&req->f.iocb, 0, sizeof(struct kiocb));
-	for_each_sg_page(req->sg, &sg_pg_iter, req->sg_cnt, 0) {
-		nvmet_file_init_bvec(&req->f.bvec[bv_cnt], &sg_pg_iter);
+	for_each_sg(req->sg, sg, req->sg_cnt, i) {
+		nvmet_file_init_bvec(&req->f.bvec[bv_cnt], sg);
 		len += req->f.bvec[bv_cnt].bv_len;
 		total_len += req->f.bvec[bv_cnt].bv_len;
 		bv_cnt++;
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-26  3:19   ` Ming Lei
@ 2019-03-26  7:36     ` Christoph Hellwig
  -1 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-26  7:36 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-nvme, Christoph Hellwig, Yi Zhang, Sagi Grimberg,
	Chaitanya Kulkarni, stable

Note that in many cases allocating larger pages in the other targets
would be useful.  Given that you fix the one places where we made a page
size assumption I don't see a great need to limit outselves here.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-26  7:36     ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-26  7:36 UTC (permalink / raw)


Note that in many cases allocating larger pages in the other targets
would be useful.  Given that you fix the one places where we made a page
size assumption I don't see a great need to limit outselves here.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-26  7:36     ` Christoph Hellwig
@ 2019-03-26  7:37       ` Ming Lei
  -1 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable,
	linux-nvme

On Tue, Mar 26, 2019 at 3:36 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Note that in many cases allocating larger pages in the other targets
> would be useful.  Given that you fix the one places where we made a page
> size assumption I don't see a great need to limit outselves here.

block device backed ns still needs this patch.


thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-26  7:37       ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:37 UTC (permalink / raw)


On Tue, Mar 26, 2019@3:36 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Note that in many cases allocating larger pages in the other targets
> would be useful.  Given that you fix the one places where we made a page
> size assumption I don't see a great need to limit outselves here.

block device backed ns still needs this patch.


thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] nvmet: fix computation of io vector number
  2019-03-26  3:19   ` Ming Lei
@ 2019-03-26  7:38     ` Christoph Hellwig
  -1 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-26  7:38 UTC (permalink / raw)
  To: Ming Lei
  Cc: linux-nvme, Christoph Hellwig, Yi Zhang, Sagi Grimberg,
	Chaitanya Kulkarni, stable

I'd rather see this merged with the last one (and patch 3 dropped as
it just gets removed entirely in 4) - that fixes all the issues together
and doesn't require your patch 1 as as a workaround first.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/4] nvmet: fix computation of io vector number
@ 2019-03-26  7:38     ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-26  7:38 UTC (permalink / raw)


I'd rather see this merged with the last one (and patch 3 dropped as
it just gets removed entirely in 4) - that fixes all the issues together
and doesn't require your patch 1 as as a workaround first.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-26  7:37       ` Ming Lei
@ 2019-03-26  7:39         ` Ming Lei
  -1 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable,
	linux-nvme

On Tue, Mar 26, 2019 at 3:37 PM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Tue, Mar 26, 2019 at 3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Note that in many cases allocating larger pages in the other targets
> > would be useful.  Given that you fix the one places where we made a page
> > size assumption I don't see a great need to limit outselves here.
>
> block device backed ns still needs this patch.

Also both the 2nd & 3rd fixes depend on this patch too.


Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-26  7:39         ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:39 UTC (permalink / raw)


On Tue, Mar 26, 2019@3:37 PM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Tue, Mar 26, 2019@3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Note that in many cases allocating larger pages in the other targets
> > would be useful.  Given that you fix the one places where we made a page
> > size assumption I don't see a great need to limit outselves here.
>
> block device backed ns still needs this patch.

Also both the 2nd & 3rd fixes depend on this patch too.


Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] nvmet: fix computation of io vector number
  2019-03-26  7:38     ` Christoph Hellwig
@ 2019-03-26  7:50       ` Ming Lei
  -1 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:50 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable,
	linux-nvme

On Tue, Mar 26, 2019 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> I'd rather see this merged with the last one (and patch 3 dropped as
> it just gets removed entirely in 4) - that fixes all the issues together
> and doesn't require your patch 1 as as a workaround first.

Patch 1 isn't a workaround, which is needed for stable tree, and
the other fixes won't work without patch 1.

However, we may revert patch 1 for 5.1 in which multi-page bvec
is supported.

I am fine to merge the last three in one patch since the backporting
isn't too hard for me, :-) However I guess our stable guys may prefer to
simpler fix.

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/4] nvmet: fix computation of io vector number
@ 2019-03-26  7:50       ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-26  7:50 UTC (permalink / raw)


On Tue, Mar 26, 2019@3:38 PM Christoph Hellwig <hch@lst.de> wrote:
>
> I'd rather see this merged with the last one (and patch 3 dropped as
> it just gets removed entirely in 4) - that fixes all the issues together
> and doesn't require your patch 1 as as a workaround first.

Patch 1 isn't a workaround, which is needed for stable tree, and
the other fixes won't work without patch 1.

However, we may revert patch 1 for 5.1 in which multi-page bvec
is supported.

I am fine to merge the last three in one patch since the backporting
isn't too hard for me, :-) However I guess our stable guys may prefer to
simpler fix.

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-26  7:37       ` Ming Lei
@ 2019-03-27  8:15         ` Christoph Hellwig
  -1 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:15 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, Ming Lei, Yi Zhang, Sagi Grimberg,
	Chaitanya Kulkarni, stable, linux-nvme

On Tue, Mar 26, 2019 at 03:37:36PM +0800, Ming Lei wrote:
> On Tue, Mar 26, 2019 at 3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Note that in many cases allocating larger pages in the other targets
> > would be useful.  Given that you fix the one places where we made a page
> > size assumption I don't see a great need to limit outselves here.
> 
> block device backed ns still needs this patch.

Then we need to fix that code as well.  Or even better make bio_add_page
handle the larger than page case fine if it really doesn't do that yet.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-27  8:15         ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:15 UTC (permalink / raw)


On Tue, Mar 26, 2019@03:37:36PM +0800, Ming Lei wrote:
> On Tue, Mar 26, 2019@3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Note that in many cases allocating larger pages in the other targets
> > would be useful.  Given that you fix the one places where we made a page
> > size assumption I don't see a great need to limit outselves here.
> 
> block device backed ns still needs this patch.

Then we need to fix that code as well.  Or even better make bio_add_page
handle the larger than page case fine if it really doesn't do that yet.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] nvmet: fix computation of io vector number
  2019-03-26  7:50       ` Ming Lei
@ 2019-03-27  8:15         ` Christoph Hellwig
  -1 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:15 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, Ming Lei, Yi Zhang, Sagi Grimberg,
	Chaitanya Kulkarni, stable, linux-nvme

On Tue, Mar 26, 2019 at 03:50:01PM +0800, Ming Lei wrote:
> On Tue, Mar 26, 2019 at 3:38 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > I'd rather see this merged with the last one (and patch 3 dropped as
> > it just gets removed entirely in 4) - that fixes all the issues together
> > and doesn't require your patch 1 as as a workaround first.
> 
> Patch 1 isn't a workaround, which is needed for stable tree, and
> the other fixes won't work without patch 1.
> 
> However, we may revert patch 1 for 5.1 in which multi-page bvec
> is supported.

Lets do it another way around - put the proper fix into 5.1-rc and
only apply patch 1 for stable.  No need to put something in and
immediately revert it.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/4] nvmet: fix computation of io vector number
@ 2019-03-27  8:15         ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:15 UTC (permalink / raw)


On Tue, Mar 26, 2019@03:50:01PM +0800, Ming Lei wrote:
> On Tue, Mar 26, 2019@3:38 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > I'd rather see this merged with the last one (and patch 3 dropped as
> > it just gets removed entirely in 4) - that fixes all the issues together
> > and doesn't require your patch 1 as as a workaround first.
> 
> Patch 1 isn't a workaround, which is needed for stable tree, and
> the other fixes won't work without patch 1.
> 
> However, we may revert patch 1 for 5.1 in which multi-page bvec
> is supported.

Lets do it another way around - put the proper fix into 5.1-rc and
only apply patch 1 for stable.  No need to put something in and
immediately revert it.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-27  8:15         ` Christoph Hellwig
@ 2019-03-27  8:26           ` Ming Lei
  -1 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-27  8:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ming Lei, Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni, stable,
	linux-nvme

On Wed, Mar 27, 2019 at 4:15 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Mar 26, 2019 at 03:37:36PM +0800, Ming Lei wrote:
> > On Tue, Mar 26, 2019 at 3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > Note that in many cases allocating larger pages in the other targets
> > > would be useful.  Given that you fix the one places where we made a page
> > > size assumption I don't see a great need to limit outselves here.
> >
> > block device backed ns still needs this patch.
>
> Then we need to fix that code as well.  Or even better make bio_add_page
> handle the larger than page case fine if it really doesn't do that yet.

Given loop is the only exception, this patch is exactly the fix, right?

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-27  8:26           ` Ming Lei
  0 siblings, 0 replies; 26+ messages in thread
From: Ming Lei @ 2019-03-27  8:26 UTC (permalink / raw)


On Wed, Mar 27, 2019@4:15 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Tue, Mar 26, 2019@03:37:36PM +0800, Ming Lei wrote:
> > On Tue, Mar 26, 2019@3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > Note that in many cases allocating larger pages in the other targets
> > > would be useful.  Given that you fix the one places where we made a page
> > > size assumption I don't see a great need to limit outselves here.
> >
> > block device backed ns still needs this patch.
>
> Then we need to fix that code as well.  Or even better make bio_add_page
> handle the larger than page case fine if it really doesn't do that yet.

Given loop is the only exception, this patch is exactly the fix, right?

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
  2019-03-27  8:26           ` Ming Lei
@ 2019-03-27  8:39             ` Christoph Hellwig
  -1 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:39 UTC (permalink / raw)
  To: Ming Lei
  Cc: Christoph Hellwig, Ming Lei, Yi Zhang, Sagi Grimberg,
	Chaitanya Kulkarni, stable, linux-nvme

On Wed, Mar 27, 2019 at 04:26:26PM +0800, Ming Lei wrote:
> On Wed, Mar 27, 2019 at 4:15 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Tue, Mar 26, 2019 at 03:37:36PM +0800, Ming Lei wrote:
> > > On Tue, Mar 26, 2019 at 3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> > > >
> > > > Note that in many cases allocating larger pages in the other targets
> > > > would be useful.  Given that you fix the one places where we made a page
> > > > size assumption I don't see a great need to limit outselves here.
> > >
> > > block device backed ns still needs this patch.
> >
> > Then we need to fix that code as well.  Or even better make bio_add_page
> > handle the larger than page case fine if it really doesn't do that yet.
> 
> Given loop is the only exception, this patch is exactly the fix, right?

Well, we want to be able to pass multi-page biovecs everywhere, so I think
we'd just solve this based on loop so that we can later take advantage of
it everywhere.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1
@ 2019-03-27  8:39             ` Christoph Hellwig
  0 siblings, 0 replies; 26+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:39 UTC (permalink / raw)


On Wed, Mar 27, 2019@04:26:26PM +0800, Ming Lei wrote:
> On Wed, Mar 27, 2019@4:15 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Tue, Mar 26, 2019@03:37:36PM +0800, Ming Lei wrote:
> > > On Tue, Mar 26, 2019@3:36 PM Christoph Hellwig <hch@lst.de> wrote:
> > > >
> > > > Note that in many cases allocating larger pages in the other targets
> > > > would be useful.  Given that you fix the one places where we made a page
> > > > size assumption I don't see a great need to limit outselves here.
> > >
> > > block device backed ns still needs this patch.
> >
> > Then we need to fix that code as well.  Or even better make bio_add_page
> > handle the larger than page case fine if it really doesn't do that yet.
> 
> Given loop is the only exception, this patch is exactly the fix, right?

Well, we want to be able to pass multi-page biovecs everywhere, so I think
we'd just solve this based on loop so that we can later take advantage of
it everywhere.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2019-03-27  8:40 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-26  3:19 [PATCH 0/4] nvmet: fix file & block device backed ns Ming Lei
2019-03-26  3:19 ` [PATCH 1/4] nvmet: set loop queue's segment boundary mask as PAGE_SIZE - 1 Ming Lei
2019-03-26  3:19   ` Ming Lei
2019-03-26  7:36   ` Christoph Hellwig
2019-03-26  7:36     ` Christoph Hellwig
2019-03-26  7:37     ` Ming Lei
2019-03-26  7:37       ` Ming Lei
2019-03-26  7:39       ` Ming Lei
2019-03-26  7:39         ` Ming Lei
2019-03-27  8:15       ` Christoph Hellwig
2019-03-27  8:15         ` Christoph Hellwig
2019-03-27  8:26         ` Ming Lei
2019-03-27  8:26           ` Ming Lei
2019-03-27  8:39           ` Christoph Hellwig
2019-03-27  8:39             ` Christoph Hellwig
2019-03-26  3:19 ` [PATCH 2/4] nvmet: fix computation of io vector number Ming Lei
2019-03-26  3:19   ` Ming Lei
2019-03-26  7:38   ` Christoph Hellwig
2019-03-26  7:38     ` Christoph Hellwig
2019-03-26  7:50     ` Ming Lei
2019-03-26  7:50       ` Ming Lei
2019-03-27  8:15       ` Christoph Hellwig
2019-03-27  8:15         ` Christoph Hellwig
2019-03-26  3:19 ` [PATCH 3/4] nvmet: fix computation of bvec->bv_len Ming Lei
2019-03-26  3:19   ` Ming Lei
2019-03-26  3:19 ` [PATCH 4/4] nvmet: build bvec from sg directly Ming Lei

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.