Re: can we reduce bio_set_dev overhead due to bio_associate_blkg?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Apr 09 2022 at  1:15P -0400,
Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:

> On Fri, Apr 08, 2022 at 11:42:51AM -0400, Mike Snitzer wrote:
> > I think we can achieve the goal of efficient cloning/remapping for
> > both usecases simply by splitting out the bio_set_dev() and leaving it
> > to the caller to pick which interface to use (e.g. clone vs
> > clone_and_remap).
> 
> You can just pass a NULL bdev to bio_alloc_clone/bio_init_clone.
> I've been hoping to get rid of that, but if we have a clear use case
> it will have to stay.

DM core is just using bio_alloc_clone. And bio_alloc_bioset() allows
bdev to be NULL -- so you're likely referring to that (which will skip
bio_init's bio_associate_blkg).

Circling back to earlier in this thread, Dennis and you agreed that it
doesn't make sense to have __bio_clone() do blkcg work if the clone
bio will be remapped (via bio_set_dev).  Given that, and the fact that
bio_clone_blkg_association() assumes both bios are from same bdev,
this change makes sense:

diff --git a/block/bio.c b/block/bio.c
index 7892f1108ca6..0340acc283a0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -772,14 +772,16 @@ static int __bio_clone(struct bio *bio, struct bio *bio_src, gfp_t gfp)
	bio_set_flag(bio, BIO_CLONED);
	if (bio_flagged(bio_src, BIO_THROTTLED))
		bio_set_flag(bio, BIO_THROTTLED);
-	if (bio->bi_bdev == bio_src->bi_bdev &&
-	    bio_flagged(bio_src, BIO_REMAPPED))
-		bio_set_flag(bio, BIO_REMAPPED);
	bio->bi_ioprio = bio_src->bi_ioprio;
	bio->bi_iter = bio_src->bi_iter;

-	bio_clone_blkg_association(bio, bio_src);
-	blkcg_bio_issue_init(bio);
+	if (bio->bi_bdev == bio_src->bi_bdev) {
+		if (bio_flagged(bio_src, BIO_REMAPPED))
+			bio_set_flag(bio, BIO_REMAPPED);
+
+		bio_clone_blkg_association(bio, bio_src);
+		blkcg_bio_issue_init(bio);
+	}

	if (bio_crypt_clone(bio, bio_src, gfp) < 0)
		return -ENOMEM;

Think this will fix some of the performance penalty of redundant blkcg
initialization that I reported (though like was also discussed: more
work likely needed to further optimize bio_associate_blkg).

I'll audit DM targets and test to verify my changes and will post
proper patch(es) once done.

Mike

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux