lkml.org 
[lkml]   [2021]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 2/6] block: add blk_interposer
    The 02/03/2021 11:18, Mike Snitzer wrote:
    > On Wed, Feb 03 2021 at 10:53am -0500,
    > Sergei Shtepa <sergei.shtepa@veeam.com> wrote:
    >
    > > blk_interposer allows to intercept bio requests, remap bio to another devices or add new bios.
    > >
    > > Signed-off-by: Sergei Shtepa <sergei.shtepa@veeam.com>
    > > ---
    > > block/bio.c | 2 +
    > > block/blk-core.c | 33 ++++++++++++++++
    > > block/genhd.c | 82 +++++++++++++++++++++++++++++++++++++++
    > > include/linux/blk_types.h | 6 ++-
    > > include/linux/genhd.h | 18 +++++++++
    > > 5 files changed, 139 insertions(+), 2 deletions(-)
    > >
    > > diff --git a/block/bio.c b/block/bio.c
    > > index 1f2cc1fbe283..f6f135eb84b5 100644
    > > --- a/block/bio.c
    > > +++ b/block/bio.c
    > > @@ -684,6 +684,8 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src)
    > > bio_set_flag(bio, BIO_CLONED);
    > > if (bio_flagged(bio_src, BIO_THROTTLED))
    > > bio_set_flag(bio, BIO_THROTTLED);
    > > + if (bio_flagged(bio_src, BIO_INTERPOSED))
    > > + bio_set_flag(bio, BIO_INTERPOSED);
    > > bio->bi_opf = bio_src->bi_opf;
    > > bio->bi_ioprio = bio_src->bi_ioprio;
    > > bio->bi_write_hint = bio_src->bi_write_hint;
    > > diff --git a/block/blk-core.c b/block/blk-core.c
    > > index 7663a9b94b80..c84bc42ba88b 100644
    > > --- a/block/blk-core.c
    > > +++ b/block/blk-core.c
    > > @@ -1032,6 +1032,32 @@ static blk_qc_t __submit_bio_noacct_mq(struct bio *bio)
    > > return ret;
    > > }
    > >
    > > +static blk_qc_t __submit_bio_interposed(struct bio *bio)
    > > +{
    > > + struct bio_list bio_list[2] = { };
    > > + blk_qc_t ret = BLK_QC_T_NONE;
    > > +
    > > + current->bio_list = bio_list;
    > > + if (likely(bio_queue_enter(bio) == 0)) {
    > > + struct gendisk *disk = bio->bi_disk;
    > > +
    > > + if (likely(blk_has_interposer(disk))) {
    > > + bio_set_flag(bio, BIO_INTERPOSED);
    > > + disk->interposer->ip_submit_bio(bio);
    > > + } else /* interposer was removed */
    > > + bio_list_add(&current->bio_list[0], bio);
    >
    > style nit:
    >
    > } else {
    > /* interposer was removed */
    > bio_list_add(&current->bio_list[0], bio);
    > }
    >
    > > +
    > > + blk_queue_exit(disk->queue);
    > > + }
    > > + current->bio_list = NULL;
    > > +
    > > + /* Resubmit remaining bios */
    > > + while ((bio = bio_list_pop(&bio_list[0])))
    > > + ret = submit_bio_noacct(bio);
    > > +
    > > + return ret;
    > > +}
    > > +
    > > /**
    > > * submit_bio_noacct - re-submit a bio to the block device layer for I/O
    > > * @bio: The bio describing the location in memory and on the device.
    > > @@ -1057,6 +1083,13 @@ blk_qc_t submit_bio_noacct(struct bio *bio)
    > > return BLK_QC_T_NONE;
    > > }
    > >
    > > + /*
    > > + * Checking the BIO_INTERPOSED flag is necessary so that the bio
    > > + * created by the blk_interposer do not get to it for processing.
    > > + */
    > > + if (blk_has_interposer(bio->bi_disk) &&
    > > + !bio_flagged(bio, BIO_INTERPOSED))
    > > + return __submit_bio_interposed(bio);
    > > if (!bio->bi_disk->fops->submit_bio)
    > > return __submit_bio_noacct_mq(bio);
    > > return __submit_bio_noacct(bio);
    > > diff --git a/block/genhd.c b/block/genhd.c
    > > index 419548e92d82..39785a3ef703 100644
    > > --- a/block/genhd.c
    > > +++ b/block/genhd.c
    > > @@ -30,6 +30,7 @@
    > > static struct kobject *block_depr;
    > >
    > > DECLARE_RWSEM(bdev_lookup_sem);
    > > +DEFINE_MUTEX(bdev_interposer_mutex);
    >
    > Seems you're using this mutex to protect access to disk->interposer in
    > attach/detach. This is to prevent attach/detach races to same device?

    Yes. There is a probability of 0.00...01% that two different modules will
    try to attach/detach to the same disk at the same time.
    Since the attach/detach operation is infrequent, using mutex is quite appropriate.
    >
    > Thankfully attach/detach isn't in the bio submission fast path but it'd
    > be helpful to document what this mutex is protecting).

    I'll think about the name of this mutex and add a comment.

    >
    > A storm of attach or detach will all hit this global mutex though...
    >
    > Mike
    >

    Thank you for the review.
    I am very interested in your opinion about [PATCH v4 4/6] and [PATCH v4 5/6].
    However, the kernel test robot has already found something there on sparc.

    --
    Sergei Shtepa
    Veeam Software developer.

    \
     
     \ /
      Last update: 2021-02-04 11:16    [W:5.368 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site