Re: [PATCH 10/11] rnbd-srv: use bdev_discard_alignment
On Mon, Apr 18, 2022 at 6:53 AM Christoph Hellwig wrote:
>
> Use bdev_discard_alignment to calculate the correct discard alignment
> offset even for partitions instead of just looking at the queue limit.
>
> Signed-off-by: Christoph Hellwig
Thx!
Acked-by: Jack Wang
> ---
> drivers/block/rnbd/rnbd-srv-dev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
> b/drivers/block/rnbd/rnbd-srv-dev.h
> index d080a0de59225..4309e52524691 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.h
> +++ b/drivers/block/rnbd/rnbd-srv-dev.h
> @@ -59,7 +59,7 @@ static inline int rnbd_dev_get_discard_granularity(const
> struct rnbd_dev *dev)
>
> static inline int rnbd_dev_get_discard_alignment(const struct rnbd_dev *dev)
> {
> - return bdev_get_queue(dev->bdev)->limits.discard_alignment;
> + return bdev_discard_alignment(dev->bdev);
> }
>
> #endif /* RNBD_SRV_DEV_H */
> --
> 2.30.2
>
Re: [PATCH 11/19] rnbd-srv: remove struct rnbd_dev_blk_io
On Mon, Jan 24, 2022 at 10:11 AM Christoph Hellwig wrote:
>
> Only the priv field of rnbd_dev_blk_io is used, so store the value of
> that in bio->bi_private directly and remove the entire bio_set overhead.
>
> Signed-off-by: Christoph Hellwig
Reviewed-by: Jack Wang
Thanks!
> ---
> drivers/block/rnbd/rnbd-srv-dev.c | 4 +---
> drivers/block/rnbd/rnbd-srv-dev.h | 13 ++---
> drivers/block/rnbd/rnbd-srv.c | 27 ---
> drivers/block/rnbd/rnbd-srv.h | 1 -
> 4 files changed, 7 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
> b/drivers/block/rnbd/rnbd-srv-dev.c
> index 98d3e591a0885..c5d0a03911659 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.c
> +++ b/drivers/block/rnbd/rnbd-srv-dev.c
> @@ -12,8 +12,7 @@
> #include "rnbd-srv-dev.h"
> #include "rnbd-log.h"
>
> -struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> - struct bio_set *bs)
> +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags)
> {
> struct rnbd_dev *dev;
> int ret;
> @@ -30,7 +29,6 @@ struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t
> flags,
>
> dev->blk_open_flags = flags;
> bdevname(dev->bdev, dev->name);
> - dev->ibd_bio_set = bs;
>
> return dev;
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
> b/drivers/block/rnbd/rnbd-srv-dev.h
> index 1a14ece0be726..2c3df02b5e8ec 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.h
> +++ b/drivers/block/rnbd/rnbd-srv-dev.h
> @@ -14,25 +14,16 @@
>
> struct rnbd_dev {
> struct block_device *bdev;
> - struct bio_set *ibd_bio_set;
> fmode_t blk_open_flags;
> charname[BDEVNAME_SIZE];
> };
>
> -struct rnbd_dev_blk_io {
> - struct rnbd_dev *dev;
> - void *priv;
> - /* have to be last member for front_pad usage of bioset_init */
> - struct bio bio;
> -};
> -
> /**
> * rnbd_dev_open() - Open a device
> + * @path: path to open
> * @flags: open flags
> - * @bs:bio_set to use during block io,
> */
> -struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> - struct bio_set *bs);
> +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags);
>
> /**
> * rnbd_dev_close() - Close a device
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 6d228af1dcc35..ff9b389976078 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -116,9 +116,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
> *srv_sess)
>
> 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));
> + rnbd_endio(bio->bi_private, blk_status_to_errno(bio->bi_status));
> bio_put(bio);
> }
>
> @@ -131,7 +129,6 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
> struct rnbd_srv_sess_dev *sess_dev;
> u32 dev_id;
> int err;
> - struct rnbd_dev_blk_io *io;
> struct bio *bio;
> short prio;
>
> @@ -152,7 +149,7 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
> priv->sess_dev = sess_dev;
> priv->id = id;
>
> - bio = bio_alloc_bioset(GFP_KERNEL, 1,
> sess_dev->rnbd_dev->ibd_bio_set);
> + bio = bio_alloc(GFP_KERNEL, 1);
> 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");
> @@ -160,12 +157,8 @@ static int process_rdma(struct rnbd_srv_session
> *srv_sess,
> goto bio_put;
> }
>
> - io = container_of(bio, struct rnbd_dev_blk_io, bio);
> - io->dev = sess_dev->rnbd_dev;
> - io->priv = priv;
> -
> bio->bi_end_io = rnbd_dev_bi_end_io;
> - bio->bi_private = io;
> + bio->bi_private = priv;
> bio->bi_opf = rnbd_to_bio_flags(le32_to_cpu(msg->rw));
> bio->bi_iter.bi_sector = le64_to_cpu(msg->sector);
> bio->bi_iter.bi_size = le32_to_cpu(msg->bi_size);
> @@ -260,7 +253,6 @@ static void destroy_sess(struct rnbd_srv_session
> *srv_sess)
>
> out:
> xa_destroy(&srv_sess->index_idr);
> - bioset_exit(&srv_sess->sess_bio_set);
>
> pr_info("RTRS Session %s disconnected\n", srv_sess->sessname);
>
> @@ -289,16 +281,6 @@ static int create_sess(struct rtrs_srv_sess *rtrs)
> return -ENOMEM;
>
> srv_sess->queue_depth = rtrs_srv_get_queue_depth(rtrs);
> - err = bioset_init(&srv_sess->sess_bio_set, srv_sess->queue_depth,
> - offsetof(struct rnbd_dev_blk_io, bio),
> - BIOSET_NEED_BVECS);
> - if (err) {
> - pr_err("Allocating srv_session for path %s failed\n",
> -
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 11/19] rnbd-src: remove struct rnbd_dev_blk_io
On Tue, Jan 18, 2022 at 8:20 AM Christoph Hellwig wrote:
>
> Only the priv field of rnbd_dev_blk_io is used, so store the value of
> that in bio->bi_private directly and remove the entire bio_set overhead.
>
> Signed-off-by: Christoph Hellwig
there is one typo in the subject line, should be rnbd-srv.
> ---
> drivers/block/rnbd/rnbd-srv-dev.c | 4 +---
> drivers/block/rnbd/rnbd-srv-dev.h | 13 ++---
> drivers/block/rnbd/rnbd-srv.c | 30 +-
> drivers/block/rnbd/rnbd-srv.h | 1 -
> 4 files changed, 8 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.c
> b/drivers/block/rnbd/rnbd-srv-dev.c
> index 98d3e591a0885..c5d0a03911659 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.c
> +++ b/drivers/block/rnbd/rnbd-srv-dev.c
> @@ -12,8 +12,7 @@
> #include "rnbd-srv-dev.h"
> #include "rnbd-log.h"
>
> -struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> - struct bio_set *bs)
> +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags)
> {
> struct rnbd_dev *dev;
> int ret;
> @@ -30,7 +29,6 @@ struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t
> flags,
>
> dev->blk_open_flags = flags;
> bdevname(dev->bdev, dev->name);
> - dev->ibd_bio_set = bs;
>
> return dev;
>
> diff --git a/drivers/block/rnbd/rnbd-srv-dev.h
> b/drivers/block/rnbd/rnbd-srv-dev.h
> index 1a14ece0be726..2c3df02b5e8ec 100644
> --- a/drivers/block/rnbd/rnbd-srv-dev.h
> +++ b/drivers/block/rnbd/rnbd-srv-dev.h
> @@ -14,25 +14,16 @@
>
> struct rnbd_dev {
> struct block_device *bdev;
> - struct bio_set *ibd_bio_set;
> fmode_t blk_open_flags;
> charname[BDEVNAME_SIZE];
> };
>
> -struct rnbd_dev_blk_io {
> - struct rnbd_dev *dev;
> - void *priv;
> - /* have to be last member for front_pad usage of bioset_init */
> - struct bio bio;
> -};
> -
> /**
> * rnbd_dev_open() - Open a device
> + * @path: path to open
> * @flags: open flags
> - * @bs:bio_set to use during block io,
> */
> -struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags,
> - struct bio_set *bs);
> +struct rnbd_dev *rnbd_dev_open(const char *path, fmode_t flags);
>
> /**
> * rnbd_dev_close() - Close a device
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 65c670e96075b..b1ac1414b56d5 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -116,9 +116,7 @@ rnbd_get_sess_dev(int dev_id, struct rnbd_srv_session
> *srv_sess)
>
> 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));
> + rnbd_endio(bio->bi_private, blk_status_to_errno(bio->bi_status));
> bio_put(bio);
> }
>
> @@ -131,7 +129,6 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
> struct rnbd_srv_sess_dev *sess_dev;
> u32 dev_id;
> int err;
> - struct rnbd_dev_blk_io *io;
> struct bio *bio;
> short prio;
>
> @@ -152,20 +149,16 @@ static int process_rdma(struct rnbd_srv_session
> *srv_sess,
> priv->sess_dev = sess_dev;
> priv->id = id;
>
> - bio = bio_alloc_bioset(GFP_KERNEL, 1,
> sess_dev->rnbd_dev->ibd_bio_set);
> + bio = bio_alloc(GFP_KERNEL, 1);
> 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;
> + goto bio_put;
ok, bio_put is used here, I think it's better the move to patch 10.
> }
>
> - io = container_of(bio, struct rnbd_dev_blk_io, bio);
> - io->dev = sess_dev->rnbd_dev;
> - io->priv = priv;
> -
> bio->bi_end_io = rnbd_dev_bi_end_io;
> - bio->bi_private = io;
> + bio->bi_private = priv;
> bio->bi_opf = rnbd_to_bio_flags(le32_to_cpu(msg->rw));
> bio->bi_iter.bi_sector = le64_to_cpu(msg->sector);
> bio->bi_iter.bi_size = le32_to_cpu(msg->bi_size);
> @@ -180,7 +173,6 @@ static int process_rdma(struct rnbd_srv_session *srv_sess,
>
> bio_put:
> bio_put(bio);
> -sess_dev_put:
> rnbd_put_sess_dev(sess_dev);
> err:
> kfree(priv);
> @@ -261,7 +253,6 @@ static void destroy_sess(struct rnbd_srv_session
> *srv_sess)
>
> out:
> xa_destroy(&srv_sess->index_idr);
> - bioset_exit(&srv_sess->sess_bio_set);
>
> pr_info("RTRS Session %s disconnected\n", srv_sess->sessname);
>
> @@ -290,16 +281,6 @@ static int create_sess(struct rtrs_srv_sess *rtrs)
> return -ENOMEM;
>
> srv_sess->queue_depth = rtrs_srv_get_queue_depth(rtrs);
> -
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
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,
Re: [PATCH 23/30] rnbd: use blk_mq_alloc_disk and blk_cleanup_disk
On Wed, Jun 2, 2021 at 8:55 AM Christoph Hellwig wrote:
>
> Use blk_mq_alloc_disk and blk_cleanup_disk to simplify the gendisk and
> request_queue allocation.
>
> Signed-off-by: Christoph Hellwig
> ---
> drivers/block/rnbd/rnbd-clt.c | 35 ---
> 1 file changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index c604a402cd5c..f4fa45d24c0b 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -1353,18 +1353,6 @@ static void rnbd_init_mq_hw_queues(struct rnbd_clt_dev
> *dev)
> }
> }
>
> -static int setup_mq_dev(struct rnbd_clt_dev *dev)
> -{
> - dev->queue = blk_mq_init_queue(&dev->sess->tag_set);
> - if (IS_ERR(dev->queue)) {
> - rnbd_clt_err(dev, "Initializing multiqueue queue failed, err:
> %ld\n",
> - PTR_ERR(dev->queue));
> - return PTR_ERR(dev->queue);
> - }
> - rnbd_init_mq_hw_queues(dev);
> - return 0;
> -}
> -
> static void setup_request_queue(struct rnbd_clt_dev *dev)
> {
> blk_queue_logical_block_size(dev->queue, dev->logical_block_size);
> @@ -1393,13 +1381,13 @@ static void setup_request_queue(struct rnbd_clt_dev
> *dev)
> blk_queue_io_opt(dev->queue, dev->sess->max_io_size);
> blk_queue_virt_boundary(dev->queue, SZ_4K - 1);
> blk_queue_write_cache(dev->queue, dev->wc, dev->fua);
> - dev->queue->queuedata = dev;
> }
>
> static void rnbd_clt_setup_gen_disk(struct rnbd_clt_dev *dev, int idx)
> {
> dev->gd->major = rnbd_client_major;
> dev->gd->first_minor= idx << RNBD_PART_BITS;
> + dev->gd->minors = 1 << RNBD_PART_BITS;
> dev->gd->fops = &rnbd_client_ops;
> dev->gd->queue = dev->queue;
> dev->gd->private_data = dev;
> @@ -1426,24 +1414,18 @@ static void rnbd_clt_setup_gen_disk(struct
> rnbd_clt_dev *dev, int idx)
>
> static int rnbd_client_setup_device(struct rnbd_clt_dev *dev)
> {
> - int err, idx = dev->clt_device_id;
> + int idx = dev->clt_device_id;
>
> dev->size = dev->nsectors * dev->logical_block_size;
>
> - err = setup_mq_dev(dev);
> - if (err)
> - return err;
> + dev->gd = blk_mq_alloc_disk(&dev->sess->tag_set, dev);
> + if (IS_ERR(dev->gd))
> + return PTR_ERR(dev->gd);
> + dev->queue = dev->gd->queue;
> + rnbd_init_mq_hw_queues(dev);
>
> setup_request_queue(dev);
> -
> - dev->gd = alloc_disk_node(1 << RNBD_PART_BITS, NUMA_NO_NODE);
> - if (!dev->gd) {
> - blk_cleanup_queue(dev->queue);
> - return -ENOMEM;
> - }
> -
> rnbd_clt_setup_gen_disk(dev, idx);
> -
> return 0;
> }
>
> @@ -1650,8 +1632,7 @@ struct rnbd_clt_dev *rnbd_clt_map_device(const char
> *sessname,
> static void destroy_gen_disk(struct rnbd_clt_dev *dev)
> {
> del_gendisk(dev->gd);
> - blk_cleanup_queue(dev->queue);
> - put_disk(dev->gd);
> + blk_cleanup_disk(dev->gd);
> }
>
> static void destroy_sysfs(struct rnbd_clt_dev *dev,
> --
> 2.30.2
Looks good to me, thx!
Reviewed-by: Jack Wang
>
Re: [PATCH 17/24] rbd: use set_capacity_and_notify
On Wed, Nov 11, 2020 at 10:55 AM Ilya Dryomov wrote:
>
> On Wed, Nov 11, 2020 at 9:27 AM Christoph Hellwig wrote:
> >
> > Use set_capacity_and_notify to set the size of both the disk and block
> > device. This also gets the uevent notifications for the resize for free.
> >
> > Signed-off-by: Christoph Hellwig
> > Acked-by: Jack Wang
> > ---
> > drivers/block/rbd.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> > index f84128abade319..b7a194ffda55b4 100644
> > --- a/drivers/block/rbd.c
> > +++ b/drivers/block/rbd.c
> > @@ -4920,8 +4920,7 @@ static void rbd_dev_update_size(struct rbd_device
> > *rbd_dev)
> > !test_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags)) {
> > size = (sector_t)rbd_dev->mapping.size / SECTOR_SIZE;
> > dout("setting size to %llu sectors", (unsigned long
> > long)size);
> > - set_capacity(rbd_dev->disk, size);
> > - revalidate_disk_size(rbd_dev->disk, true);
> > + set_capacity_and_notify(rbd_dev->disk, size);
> > }
> > }
> >
> > --
> > 2.28.0
> >
>
> Hi Christoph,
>
> The Acked-by is wrong here. I acked this patch (17/24, rbd), and Jack
> acked the next one (18/24, rnbd).
right. :)
>
> Thanks,
>
> Ilya
Re: [PATCH 18/24] rnbd: use set_capacity_and_notify
On Fri, Nov 6, 2020 at 8:04 PM Christoph Hellwig wrote: > > Use set_capacity_and_notify to set the size of both the disk and block > device. This also gets the uevent notifications for the resize for free. > > Signed-off-by: Christoph Hellwig Thanks, Christoph! Acked-by: Jack Wang > --- > drivers/block/rnbd/rnbd-clt.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c > index 8b2411ccbda97c..bb13d7dd195a08 100644 > --- a/drivers/block/rnbd/rnbd-clt.c > +++ b/drivers/block/rnbd/rnbd-clt.c > @@ -100,8 +100,7 @@ static int rnbd_clt_change_capacity(struct rnbd_clt_dev > *dev, > rnbd_clt_info(dev, "Device size changed from %zu to %zu sectors\n", >dev->nsectors, new_nsectors); > dev->nsectors = new_nsectors; > - set_capacity(dev->gd, dev->nsectors); > - revalidate_disk_size(dev->gd, true); > + set_capacity_and_notify(dev->gd, dev->nsectors); > return 0; > } > > -- > 2.28.0 >
