The 02/03/2021 11:18, Mike Snitzer wrote:
> On Wed, Feb 03 2021 at 10:53am -0500,
> Sergei Shtepa <sergei.shtepa@xxxxxxxxx> wrote:
>
> > blk_interposer allows to intercept bio requests, remap bio to another devices or add new bios.
> >
> > Signed-off-by: Sergei Shtepa <sergei.shtepa@xxxxxxxxx>
> > ---
> > 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(¤t->bio_list[0], bio);
>
> style nit:
>
> } else {
> /* interposer was removed */
> bio_list_add(¤t->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.
