[PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
The memory mapped in process_rdma is contiguous, so there is no need
to loop over bio_add_page. Remove rnbd_bio_map_kern and just open code
the bio allocation and mapping in the caller.
Signed-off-by: Christoph Hellwig
Reviewed-by: Jack Wang
Tested-by: Jack Wang
---
drivers/block/rnbd/rnbd-srv-dev.c | 57 ---
drivers/block/rnbd/rnbd-srv-dev.h | 5 ---
drivers/block/rnbd/rnbd-srv.c | 23 +
3 files changed, 16 insertions(+), 69 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
b/drivers/block/rnbd/rnbd-srv-dev.c
index b241a099aeae2..98d3e591a0885 100644
--- a/drivers/block/rnbd/rnbd-srv-dev.c
+++ b/drivers/block/rnbd/rnbd-srv-dev.c
@@ -44,60 +44,3 @@ void rnbd_dev_close(struct rnbd_dev *dev)
blkdev_put(dev->bdev, dev->blk_open_flags);
kfree(dev);
}
-
-void rnbd_dev_bi_end_io(struct bio *bio)
-{
- struct rnbd_dev_blk_io *io = bio->bi_private;
-
- rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
- bio_put(bio);
-}
-
-/**
- * rnbd_bio_map_kern - map kernel address into bio
- * @data: pointer to buffer to map
- * @bs: bio_set to use.
- * @len: length in bytes
- * @gfp_mask: allocation flags for bio allocation
- *
- * Map the kernel address into a bio suitable for io to a block
- * device. Returns an error pointer in case of error.
- */
-struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
- unsigned int len, gfp_t gfp_mask)
-{
- unsigned long kaddr = (unsigned long)data;
- unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
- unsigned long start = kaddr >> PAGE_SHIFT;
- const int nr_pages = end - start;
- int offset, i;
- struct bio *bio;
-
- bio = bio_alloc_bioset(gfp_mask, nr_pages, bs);
- if (!bio)
- return ERR_PTR(-ENOMEM);
-
- offset = offset_in_page(kaddr);
- for (i = 0; i < nr_pages; i++) {
- unsigned int bytes = PAGE_SIZE - offset;
-
- if (len <= 0)
- break;
-
- if (bytes > len)
- bytes = len;
-
- if (bio_add_page(bio, virt_to_page(data), bytes,
- offset) < bytes) {
- /* we don't support partial mappings */
- bio_put(bio);
- return ERR_PTR(-EINVAL);
- }
-
- data += bytes;
- len -= bytes;
- offset = 0;
- }
-
- return bio;
-}
diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
b/drivers/block/rnbd/rnbd-srv-dev.h
index 0eb23850afb95..1a14ece0be726 100644
--- a/drivers/block/rnbd/rnbd-srv-dev.h
+++ b/drivers/block/rnbd/rnbd-srv-dev.h
@@ -41,11 +41,6 @@ void rnbd_dev_close(struct rnbd_dev *dev);
void rnbd_endio(void *priv, int error);
-void rnbd_dev_bi_end_io(struct bio *bio);
-
-struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
- unsigned int len, gfp_t gfp_mask);
-
static inline int rnbd_dev_get_max_segs(const struct rnbd_dev *dev)
{
return queue_max_segments(bdev_get_queue(dev->bdev));
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 1ee808fc600cf..6d228af1dcc35 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -114,6 +114,14 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
*srv_sess)
return sess_dev;
}
+static void rnbd_dev_bi_end_io(struct bio *bio)
+{
+ struct rnbd_dev_blk_io *io = bio->bi_private;
+
+ rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
+ bio_put(bio);
+}
+
static int process_rdma(struct rnbd_srv_session *srv_sess,
struct rtrs_srv_op *id, void *data, u32 datalen,
const void *usr, size_t usrlen)
@@ -144,12 +152,12 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
priv->sess_dev = sess_dev;
priv->id = id;
- /* Generate bio with pages pointing to the rdma buffer */
- bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set, datalen,
GFP_KERNEL);
- if (IS_ERR(bio)) {
- err = PTR_ERR(bio);
- rnbd_srv_err(sess_dev, "Failed to generate bio, err: %d\n",
err);
- goto sess_dev_put;
+ bio = bio_alloc_bioset(GFP_KERNEL, 1, sess_dev->rnbd_dev->ibd_bio_set);
+ if (bio_add_page(bio, virt_to_page(data), datalen,
+ offset_in_page(data)) != datalen) {
+ rnbd_srv_err(sess_dev, "Failed to map data to bio\n");
+ err = -EINVAL;
+ goto bio_put;
}
io = container_of(bio, struct rnbd_dev_blk_io, bio);
@@ -170,7 +178,8 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
return 0;
-sess_dev_put:
+bio_put:
+ bio_put(bio);
rnbd_put_sess_dev(sess_dev);
err:
Re: [PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
On Thu, Jan 20, 2022 at 9:37 AM Christoph Hellwig wrote: > > On Wed, Jan 19, 2022 at 01:20:54AM +0100, Jinpu Wang wrote: > > this changes lead to IO error all the time, because bio_add_page return len. > > We need if (bio_add_page(bio, virt_to_page(data), datalen, > > offset_in_page(data)) < datalen) > > Does this version look good to you? > > http://git.infradead.org/users/hch/block.git/commitdiff/62adb08e765b889dd8db4227cad33a710e36d631 Yes, lgtm, thank you! Reviewed-by: Jack Wang Tested-by: Jack Wang
Re: [PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
On Wed, Jan 19, 2022 at 01:20:54AM +0100, Jinpu Wang wrote: > this changes lead to IO error all the time, because bio_add_page return len. > We need if (bio_add_page(bio, virt_to_page(data), datalen, > offset_in_page(data)) < datalen) Does this version look good to you? http://git.infradead.org/users/hch/block.git/commitdiff/62adb08e765b889dd8db4227cad33a710e36d631
Re: [PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
On Wed, Jan 19, 2022 at 1:20 AM Jinpu Wang wrote:
>
> Hi Christoph,
>
> Thanks for the patch.
>
> On Tue, Jan 18, 2022 at 8:20 AM Christoph Hellwig wrote:
> >
> > The memory mapped in process_rdma is contiguous, so there is no need
> > to loop over bio_add_page. Remove rnbd_bio_map_kern and just open code
> > the bio allocation and mapping in the caller.
> >
> > Signed-off-by: Christoph Hellwig
> > ---
> > drivers/block/rnbd/rnbd-srv-dev.c | 57 ---
> > drivers/block/rnbd/rnbd-srv-dev.h | 5 ---
> > drivers/block/rnbd/rnbd-srv.c | 20 ---
> > 3 files changed, 15 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
> > b/drivers/block/rnbd/rnbd-srv-dev.c
> > index b241a099aeae2..98d3e591a0885 100644
> > --- a/drivers/block/rnbd/rnbd-srv-dev.c
> > +++ b/drivers/block/rnbd/rnbd-srv-dev.c
> > @@ -44,60 +44,3 @@ void rnbd_dev_close(struct rnbd_dev *dev)
> > blkdev_put(dev->bdev, dev->blk_open_flags);
> > kfree(dev);
> > }
> > -
> > -void rnbd_dev_bi_end_io(struct bio *bio)
> > -{
> > - struct rnbd_dev_blk_io *io = bio->bi_private;
> > -
> > - rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
> > - bio_put(bio);
> > -}
> > -
> > -/**
> > - * rnbd_bio_map_kern - map kernel address into bio
> > - * @data: pointer to buffer to map
> > - * @bs: bio_set to use.
> > - * @len: length in bytes
> > - * @gfp_mask: allocation flags for bio allocation
> > - *
> > - * Map the kernel address into a bio suitable for io to a block
> > - * device. Returns an error pointer in case of error.
> > - */
> > -struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
> > - unsigned int len, gfp_t gfp_mask)
> > -{
> > - unsigned long kaddr = (unsigned long)data;
> > - unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > - unsigned long start = kaddr >> PAGE_SHIFT;
> > - const int nr_pages = end - start;
> > - int offset, i;
> > - struct bio *bio;
> > -
> > - bio = bio_alloc_bioset(gfp_mask, nr_pages, bs);
> > - if (!bio)
> > - return ERR_PTR(-ENOMEM);
> > -
> > - offset = offset_in_page(kaddr);
> > - for (i = 0; i < nr_pages; i++) {
> > - unsigned int bytes = PAGE_SIZE - offset;
> > -
> > - if (len <= 0)
> > - break;
> > -
> > - if (bytes > len)
> > - bytes = len;
> > -
> > - if (bio_add_page(bio, virt_to_page(data), bytes,
> > - offset) < bytes) {
> > - /* we don't support partial mappings */
> > - bio_put(bio);
> > - return ERR_PTR(-EINVAL);
> > - }
> > -
> > - data += bytes;
> > - len -= bytes;
> > - offset = 0;
> > - }
> > -
> > - return bio;
> > -}
> > diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
> > b/drivers/block/rnbd/rnbd-srv-dev.h
> > index 0eb23850afb95..1a14ece0be726 100644
> > --- a/drivers/block/rnbd/rnbd-srv-dev.h
> > +++ b/drivers/block/rnbd/rnbd-srv-dev.h
> > @@ -41,11 +41,6 @@ void rnbd_dev_close(struct rnbd_dev *dev);
> >
> > void rnbd_endio(void *priv, int error);
> >
> > -void rnbd_dev_bi_end_io(struct bio *bio);
> > -
> > -struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
> > - unsigned int len, gfp_t gfp_mask);
> > -
> > static inline int rnbd_dev_get_max_segs(const struct rnbd_dev *dev)
> > {
> > return queue_max_segments(bdev_get_queue(dev->bdev));
> > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> > index 1ee808fc600cf..65c670e96075b 100644
> > --- a/drivers/block/rnbd/rnbd-srv.c
> > +++ b/drivers/block/rnbd/rnbd-srv.c
> > @@ -114,6 +114,14 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
> > *srv_sess)
> > return sess_dev;
> > }
> >
> > +static void rnbd_dev_bi_end_io(struct bio *bio)
> > +{
> > + struct rnbd_dev_blk_io *io = bio->bi_private;
> > +
> > + rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
> > + bio_put(bio);
> > +}
> > +
> > static int process_rdma(struct rnbd_srv_session *srv_sess,
> > struct rtrs_srv_op *id, void *data, u32 datalen,
> > const void *usr, size_t usrlen)
> > @@ -144,11 +152,11 @@ static int process_rdma(struct rnbd_srv_session
> > *srv_sess,
> > priv->sess_dev = sess_dev;
> > priv->id = id;
> >
> > - /* Generate bio with pages pointing to the rdma buffer */
> > - bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set,
> > datalen, GFP_KERNEL);
> > - if (IS_ERR(bio)) {
> > - err = PTR_ERR(bio);
> > - rnbd_srv_err(sess_dev, "Failed to generate bio, err: %d\n",
> > err);
> > + bio = bio_a
Re: [PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
Hi Christoph,
Thanks for the patch.
On Tue, Jan 18, 2022 at 8:20 AM Christoph Hellwig wrote:
>
> The memory mapped in process_rdma is contiguous, so there is no need
> to loop over bio_add_page. Remove rnbd_bio_map_kern and just open code
> the bio allocation and mapping in the caller.
>
> Signed-off-by: Christoph Hellwig
> ---
> drivers/block/rnbd/rnbd-srv-dev.c | 57 ---
> drivers/block/rnbd/rnbd-srv-dev.h | 5 ---
> drivers/block/rnbd/rnbd-srv.c | 20 ---
> 3 files changed, 15 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
> b/drivers/block/rnbd/rnbd-srv-dev.c
> index b241a099aeae2..98d3e591a0885 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.c
> +++ b/drivers/block/rnbd/rnbd-srv-dev.c
> @@ -44,60 +44,3 @@ void rnbd_dev_close(struct rnbd_dev *dev)
> blkdev_put(dev->bdev, dev->blk_open_flags);
> kfree(dev);
> }
> -
> -void rnbd_dev_bi_end_io(struct bio *bio)
> -{
> - struct rnbd_dev_blk_io *io = bio->bi_private;
> -
> - rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
> - bio_put(bio);
> -}
> -
> -/**
> - * rnbd_bio_map_kern - map kernel address into bio
> - * @data: pointer to buffer to map
> - * @bs: bio_set to use.
> - * @len: length in bytes
> - * @gfp_mask: allocation flags for bio allocation
> - *
> - * Map the kernel address into a bio suitable for io to a block
> - * device. Returns an error pointer in case of error.
> - */
> -struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
> - unsigned int len, gfp_t gfp_mask)
> -{
> - unsigned long kaddr = (unsigned long)data;
> - unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> - unsigned long start = kaddr >> PAGE_SHIFT;
> - const int nr_pages = end - start;
> - int offset, i;
> - struct bio *bio;
> -
> - bio = bio_alloc_bioset(gfp_mask, nr_pages, bs);
> - if (!bio)
> - return ERR_PTR(-ENOMEM);
> -
> - offset = offset_in_page(kaddr);
> - for (i = 0; i < nr_pages; i++) {
> - unsigned int bytes = PAGE_SIZE - offset;
> -
> - if (len <= 0)
> - break;
> -
> - if (bytes > len)
> - bytes = len;
> -
> - if (bio_add_page(bio, virt_to_page(data), bytes,
> - offset) < bytes) {
> - /* we don't support partial mappings */
> - bio_put(bio);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - data += bytes;
> - len -= bytes;
> - offset = 0;
> - }
> -
> - return bio;
> -}
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
> b/drivers/block/rnbd/rnbd-srv-dev.h
> index 0eb23850afb95..1a14ece0be726 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.h
> +++ b/drivers/block/rnbd/rnbd-srv-dev.h
> @@ -41,11 +41,6 @@ void rnbd_dev_close(struct rnbd_dev *dev);
>
> void rnbd_endio(void *priv, int error);
>
> -void rnbd_dev_bi_end_io(struct bio *bio);
> -
> -struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
> - unsigned int len, gfp_t gfp_mask);
> -
> static inline int rnbd_dev_get_max_segs(const struct rnbd_dev *dev)
> {
> return queue_max_segments(bdev_get_queue(dev->bdev));
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 1ee808fc600cf..65c670e96075b 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -114,6 +114,14 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
> *srv_sess)
> return sess_dev;
> }
>
> +static void rnbd_dev_bi_end_io(struct bio *bio)
> +{
> + struct rnbd_dev_blk_io *io = bio->bi_private;
> +
> + rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
> + bio_put(bio);
> +}
> +
> static int process_rdma(struct rnbd_srv_session *srv_sess,
> struct rtrs_srv_op *id, void *data, u32 datalen,
> const void *usr, size_t usrlen)
> @@ -144,11 +152,11 @@ static int process_rdma(struct rnbd_srv_session
> *srv_sess,
> priv->sess_dev = sess_dev;
> priv->id = id;
>
> - /* Generate bio with pages pointing to the rdma buffer */
> - bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set,
> datalen, GFP_KERNEL);
> - if (IS_ERR(bio)) {
> - err = PTR_ERR(bio);
> - rnbd_srv_err(sess_dev, "Failed to generate bio, err: %d\n",
> err);
> + bio = bio_alloc_bioset(GFP_KERNEL, 1,
> sess_dev->rnbd_dev->ibd_bio_set);
> + if (bio_add_page(bio, virt_to_page(data), datalen,
> + offset_in_page(data))) {
this changes lead to IO error all the time, because bio_add_page return len.
We need if (bio_add_page(bio, virt_to_page(data), datalen,
[PATCH 10/19] rnbd-srv: simplify bio mapping in process_rdma
The memory mapped in process_rdma is contiguous, so there is no need
to loop over bio_add_page. Remove rnbd_bio_map_kern and just open code
the bio allocation and mapping in the caller.
Signed-off-by: Christoph Hellwig
---
drivers/block/rnbd/rnbd-srv-dev.c | 57 ---
drivers/block/rnbd/rnbd-srv-dev.h | 5 ---
drivers/block/rnbd/rnbd-srv.c | 20 ---
3 files changed, 15 insertions(+), 67 deletions(-)
diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
b/drivers/block/rnbd/rnbd-srv-dev.c
index b241a099aeae2..98d3e591a0885 100644
--- a/drivers/block/rnbd/rnbd-srv-dev.c
+++ b/drivers/block/rnbd/rnbd-srv-dev.c
@@ -44,60 +44,3 @@ void rnbd_dev_close(struct rnbd_dev *dev)
blkdev_put(dev->bdev, dev->blk_open_flags);
kfree(dev);
}
-
-void rnbd_dev_bi_end_io(struct bio *bio)
-{
- struct rnbd_dev_blk_io *io = bio->bi_private;
-
- rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
- bio_put(bio);
-}
-
-/**
- * rnbd_bio_map_kern - map kernel address into bio
- * @data: pointer to buffer to map
- * @bs: bio_set to use.
- * @len: length in bytes
- * @gfp_mask: allocation flags for bio allocation
- *
- * Map the kernel address into a bio suitable for io to a block
- * device. Returns an error pointer in case of error.
- */
-struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
- unsigned int len, gfp_t gfp_mask)
-{
- unsigned long kaddr = (unsigned long)data;
- unsigned long end = (kaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
- unsigned long start = kaddr >> PAGE_SHIFT;
- const int nr_pages = end - start;
- int offset, i;
- struct bio *bio;
-
- bio = bio_alloc_bioset(gfp_mask, nr_pages, bs);
- if (!bio)
- return ERR_PTR(-ENOMEM);
-
- offset = offset_in_page(kaddr);
- for (i = 0; i < nr_pages; i++) {
- unsigned int bytes = PAGE_SIZE - offset;
-
- if (len <= 0)
- break;
-
- if (bytes > len)
- bytes = len;
-
- if (bio_add_page(bio, virt_to_page(data), bytes,
- offset) < bytes) {
- /* we don't support partial mappings */
- bio_put(bio);
- return ERR_PTR(-EINVAL);
- }
-
- data += bytes;
- len -= bytes;
- offset = 0;
- }
-
- return bio;
-}
diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
b/drivers/block/rnbd/rnbd-srv-dev.h
index 0eb23850afb95..1a14ece0be726 100644
--- a/drivers/block/rnbd/rnbd-srv-dev.h
+++ b/drivers/block/rnbd/rnbd-srv-dev.h
@@ -41,11 +41,6 @@ void rnbd_dev_close(struct rnbd_dev *dev);
void rnbd_endio(void *priv, int error);
-void rnbd_dev_bi_end_io(struct bio *bio);
-
-struct bio *rnbd_bio_map_kern(void *data, struct bio_set *bs,
- unsigned int len, gfp_t gfp_mask);
-
static inline int rnbd_dev_get_max_segs(const struct rnbd_dev *dev)
{
return queue_max_segments(bdev_get_queue(dev->bdev));
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 1ee808fc600cf..65c670e96075b 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -114,6 +114,14 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
*srv_sess)
return sess_dev;
}
+static void rnbd_dev_bi_end_io(struct bio *bio)
+{
+ struct rnbd_dev_blk_io *io = bio->bi_private;
+
+ rnbd_endio(io->priv, blk_status_to_errno(bio->bi_status));
+ bio_put(bio);
+}
+
static int process_rdma(struct rnbd_srv_session *srv_sess,
struct rtrs_srv_op *id, void *data, u32 datalen,
const void *usr, size_t usrlen)
@@ -144,11 +152,11 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
priv->sess_dev = sess_dev;
priv->id = id;
- /* Generate bio with pages pointing to the rdma buffer */
- bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set, datalen,
GFP_KERNEL);
- if (IS_ERR(bio)) {
- err = PTR_ERR(bio);
- rnbd_srv_err(sess_dev, "Failed to generate bio, err: %d\n",
err);
+ bio = bio_alloc_bioset(GFP_KERNEL, 1, sess_dev->rnbd_dev->ibd_bio_set);
+ if (bio_add_page(bio, virt_to_page(data), datalen,
+ offset_in_page(data))) {
+ rnbd_srv_err(sess_dev, "Failed to map data to bio\n");
+ err = -EINVAL;
goto sess_dev_put;
}
@@ -170,6 +178,8 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
return 0;
+bio_put:
+ bio_put(bio);
sess_dev_put:
rnbd_put_sess_dev(sess_dev);
err:
--
2.30.2
