Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1450679 > unrolled thread

[PATCH BUGFIX V4] block: add missing group association in bio-cloning functions

Started byPaolo Valente <paolo.valente@linaro.org>
First post2016-07-26 18:50 +0200
Last post2016-07-27 16:50 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH BUGFIX V4] block: add missing group association in bio-cloning functions Paolo Valente <paolo.valente@linaro.org> - 2016-07-26 18:50 +0200
    Re: [PATCH BUGFIX V4] block: add missing group association in  bio-cloning functions kbuild test robot <lkp@intel.com> - 2016-07-26 19:10 +0200
    [PATCH BUGFIX V5] block: add missing group association in bio-cloning functions Paolo Valente <paolo.valente@linaro.org> - 2016-07-27 07:30 +0200
      Re: [PATCH BUGFIX V5] block: add missing group association in  bio-cloning functions Jens Axboe <axboe@kernel.dk> - 2016-07-27 16:50 +0200

#1450679 — [PATCH BUGFIX V4] block: add missing group association in bio-cloning functions

FromPaolo Valente <paolo.valente@linaro.org>
Date2016-07-26 18:50 +0200
Subject[PATCH BUGFIX V4] block: add missing group association in bio-cloning functions
Message-ID<rZdIl-4pd-7@gated-at.bofh.it>
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.

Fixes: da2f0f74cf7d ("Btrfs: add support for blkio controllers")
Cc: stable@vger.kernel.org # v4.3+

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Reviewed-by: Nikolay Borisov <kernel@kyup.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 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 0e4aa42..4623869 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -579,6 +579,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);
 
@@ -684,6 +686,8 @@ integrity_clone:
 		}
 	}
 
+	bio_clone_blkcg_association(bio, bio_src);
+
 	return bio;
 }
 EXPORT_SYMBOL(bio_clone_bioset);
@@ -2005,6 +2009,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 75533ad..92fe3f8 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2696,12 +2696,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 9faebf7..c9403b7 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
-- 
1.9.1

[toc] | [next] | [standalone]

#1450700 — Re: [PATCH BUGFIX V4] block: add missing group association in bio-cloning functions

Fromkbuild test robot <lkp@intel.com>
Date2016-07-26 19:10 +0200
SubjectRe: [PATCH BUGFIX V4] block: add missing group association in bio-cloning functions
Message-ID<rZe1H-4Pb-3@gated-at.bofh.it>
In reply to#1450679

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test WARNING on block/for-next]
[also build test WARNING on v4.7 next-20160726]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paolo-Valente/block-add-missing-group-association-in-bio-cloning-functions/20160727-005044
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-randconfig-x015-201630 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from block/bio.c:20:0:
   include/linux/bio.h: In function 'bio_clone_blkcg_association':
>> include/linux/bio.h:480:30: warning: 'return' with a value, in function returning void
       struct bio *src) { return 0; }
                                 ^
   include/linux/bio.h:479:20: note: declared here
    static inline void bio_clone_blkcg_association(struct bio *dst,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/return +480 include/linux/bio.h

   464	void zero_fill_bio(struct bio *bio);
   465	extern struct bio_vec *bvec_alloc(gfp_t, int, unsigned long *, mempool_t *);
   466	extern void bvec_free(mempool_t *, struct bio_vec *, unsigned int);
   467	extern unsigned int bvec_nr_vecs(unsigned short idx);
   468	
   469	#ifdef CONFIG_BLK_CGROUP
   470	int bio_associate_blkcg(struct bio *bio, struct cgroup_subsys_state *blkcg_css);
   471	int bio_associate_current(struct bio *bio);
   472	void bio_disassociate_task(struct bio *bio);
   473	void bio_clone_blkcg_association(struct bio *dst, struct bio *src);
   474	#else	/* CONFIG_BLK_CGROUP */
   475	static inline int bio_associate_blkcg(struct bio *bio,
   476				struct cgroup_subsys_state *blkcg_css) { return 0; }
   477	static inline int bio_associate_current(struct bio *bio) { return -ENOENT; }
   478	static inline void bio_disassociate_task(struct bio *bio) { }
   479	static inline void bio_clone_blkcg_association(struct bio *dst,
 > 480				struct bio *src) { return 0; }
   481	#endif	/* CONFIG_BLK_CGROUP */
   482	
   483	#ifdef CONFIG_HIGHMEM
   484	/*
   485	 * remember never ever reenable interrupts between a bvec_kmap_irq and
   486	 * bvec_kunmap_irq!
   487	 */
   488	static inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1451045 — [PATCH BUGFIX V5] block: add missing group association in bio-cloning functions

FromPaolo Valente <paolo.valente@linaro.org>
Date2016-07-27 07:30 +0200
Subject[PATCH BUGFIX V5] block: add missing group association in bio-cloning functions
Message-ID<rZpzP-3Eu-5@gated-at.bofh.it>
In reply to#1450679
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.

Fixes: da2f0f74cf7d ("Btrfs: add support for blkio controllers")
Cc: stable@vger.kernel.org # v4.3+

Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
Reviewed-by: Nikolay Borisov <kernel@kyup.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
Fixed a compilation issue reported by kbuild test robot
---
 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 0e4aa42..4623869 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -579,6 +579,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);
 
@@ -684,6 +686,8 @@ integrity_clone:
 		}
 	}
 
+	bio_clone_blkcg_association(bio, bio_src);
+
 	return bio;
 }
 EXPORT_SYMBOL(bio_clone_bioset);
@@ -2005,6 +2009,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 75533ad..92fe3f8 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2696,12 +2696,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 9faebf7..75fadd2 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) { }
 #endif	/* CONFIG_BLK_CGROUP */
 
 #ifdef CONFIG_HIGHMEM
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1451311 — Re: [PATCH BUGFIX V5] block: add missing group association in bio-cloning functions

FromJens Axboe <axboe@kernel.dk>
Date2016-07-27 16:50 +0200
SubjectRe: [PATCH BUGFIX V5] block: add missing group association in bio-cloning functions
Message-ID<rZyjM-F9-3@gated-at.bofh.it>
In reply to#1451045
On 07/26/2016 11:22 PM, Paolo Valente wrote:
> 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.

Added for 4.8, thanks.

-- 
Jens Axboe

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web