Paolo Valente <paolo.vale...@linaro.org> 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 <paolo.vale...@linaro.org>
> Reviewed-by: Nikolay Borisov <ker...@kyup.com>
I think this one's golden! Thanks, Paolo!
Reviewed-by: Jeff Moyer <jmo...@redhat.com>
> ---
> 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
