From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753358AbcEMUm5 (ORCPT ); Fri, 13 May 2016 16:42:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39958 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbcEMUmz (ORCPT ); Fri, 13 May 2016 16:42:55 -0400 From: Jeff Moyer To: Paolo Valente Cc: Jens Axboe , Tejun Heo , linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, ulf.hansson@linaro.org, linus.walleij@linaro.org, broonie@kernel.org, stable@vger.kernel.org Subject: Re: [PATCH BUGFIX V4] block: add missing group association in bio-cloning functions References: <5732D47A.4090706@kyup.com> <1462958884-9457-1-git-send-email-paolo.valente@linaro.org> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Fri, 13 May 2016 16:42:52 -0400 In-Reply-To: <1462958884-9457-1-git-send-email-paolo.valente@linaro.org> (Paolo Valente's message of "Wed, 11 May 2016 11:28:04 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 13 May 2016 20:42:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Paolo Valente writes: > When a bio is cloned, the newly created bio must be associated with > the same blkcg as the original bio (if BLK_CGROUP is enabled). If > this operation is not performed, then the new bio is not associated > with any group, and the group of the current task is returned when > the group of the bio is requested. > > Depending on the cloning frequency, this may cause a large > percentage of the bios belonging to a given group to be treated > as if belonging to other groups (in most cases as if belonging to > the root group). The expected group isolation may thereby be broken. > > This commit adds the missing association in bio-cloning functions. > > Signed-off-by: Paolo Valente > Reviewed-by: Nikolay Borisov I think this one's golden! Thanks, Paolo! Reviewed-by: Jeff Moyer > --- > Tejun: I didn't add also your Ack, just because this version is slightly > different from the one you acked. > --- > block/bio.c | 15 +++++++++++++++ > fs/btrfs/extent_io.c | 6 ------ > include/linux/bio.h | 3 +++ > 3 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/block/bio.c b/block/bio.c > index 807d25e..89f517c 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -590,6 +590,8 @@ void __bio_clone_fast(struct bio *bio, struct bio *bio_src) > bio->bi_rw = bio_src->bi_rw; > bio->bi_iter = bio_src->bi_iter; > bio->bi_io_vec = bio_src->bi_io_vec; > + > + bio_clone_blkcg_association(bio, bio_src); > } > EXPORT_SYMBOL(__bio_clone_fast); > > @@ -695,6 +697,8 @@ integrity_clone: > } > } > > + bio_clone_blkcg_association(bio, bio_src); > + > return bio; > } > EXPORT_SYMBOL(bio_clone_bioset); > @@ -2016,6 +2020,17 @@ void bio_disassociate_task(struct bio *bio) > } > } > > +/** > + * bio_clone_blkcg_association - clone blkcg association from src to dst bio > + * @dst: destination bio > + * @src: source bio > + */ > +void bio_clone_blkcg_association(struct bio *dst, struct bio *src) > +{ > + if (src->bi_css) > + WARN_ON(bio_associate_blkcg(dst, src->bi_css)); > +} > + > #endif /* CONFIG_BLK_CGROUP */ > > static void __init biovec_init_slabs(void) > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index d247fc0..19f6739 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -2686,12 +2686,6 @@ struct bio *btrfs_bio_clone(struct bio *bio, gfp_t gfp_mask) > btrfs_bio->csum = NULL; > btrfs_bio->csum_allocated = NULL; > btrfs_bio->end_io = NULL; > - > -#ifdef CONFIG_BLK_CGROUP > - /* FIXME, put this into bio_clone_bioset */ > - if (bio->bi_css) > - bio_associate_blkcg(new, bio->bi_css); > -#endif > } > return new; > } > diff --git a/include/linux/bio.h b/include/linux/bio.h > index 6b7481f..16cbe59 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -527,11 +527,14 @@ extern unsigned int bvec_nr_vecs(unsigned short idx); > int bio_associate_blkcg(struct bio *bio, struct cgroup_subsys_state *blkcg_css); > int bio_associate_current(struct bio *bio); > void bio_disassociate_task(struct bio *bio); > +void bio_clone_blkcg_association(struct bio *dst, struct bio *src); > #else /* CONFIG_BLK_CGROUP */ > static inline int bio_associate_blkcg(struct bio *bio, > struct cgroup_subsys_state *blkcg_css) { return 0; } > static inline int bio_associate_current(struct bio *bio) { return -ENOENT; } > static inline void bio_disassociate_task(struct bio *bio) { } > +static inline void bio_clone_blkcg_association(struct bio *dst, > + struct bio *src) { return 0; } > #endif /* CONFIG_BLK_CGROUP */ > > #ifdef CONFIG_HIGHMEM