Skip to contentSkip to conversation

Add gfp_mask to bio_integrity_clone()

PATCH In review Jun'ichi Nomura 3 participants 4 messages Oct 30, 2008 – Oct 31, 2008

Subsystems (3)
VFS Block layer Device mapper
lore
Share & tools
lore

Jun'ichi Nomura

author
#0 lore raw

PATCH 1/1 Add gfp_mask to bio_integrity_clone()

Hi Martin,

What do you think about adding gfp_mask parameter to bio_integrity_clone() like the attached patch?

Kiyoshi Ueda and I are working on request-based device-mapper (*1) and it clones bios like the existing bio-based dm. However, since the request cloning may happen in interrupt context, GFP_ATOMIC is used in our case (*2).

(*1) http://lkml.org/lkml/2008/10/3/177
(*2) http://lkml.org/lkml/2008/10/3/188

To cope with cloning the integrity payload, we want to pass gfp_mask to bio_integrity_clone().

It also seems natural to inherit gfp_mask from bio_clone when bio_integrity_clone is called from it.
Since bio_clone() is the only caller of bio_integrity_clone() in 2.6.28-rc2 and all in-tree callers of bio_clone() are using GFP_NOIO,
the patch does nothing other than changing the interface.

Add gfp_mask parameter to bio_integrity_clone().

Stricter gfp_mask might be required for clone allocation. For example, request-based dm may clone bio in interrupt context so it has to use GFP_ATOMIC.

Signed-off-by Kiyoshi Ueda <[email protected]>
Signed-off-by Jun'ichi Nomura <[email protected]>
Cc Martin K. Petersen <[email protected]>
Cc Jens Axboe <[email protected]>
Cc Alasdair G Kergon <[email protected]>
---
 fs/bio-integrity.c  |    5 +++--
 fs/bio.c            |    2 +-
 include/linux/bio.h |    4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)
fs/bio-integrity.c +3 -2
Index: linux-2.6.28-rc2/fs/bio-integrity.c
===================================================================
@@ -681,19 +681,20 @@ EXPORT_SYMBOL(bio_integrity_split);
  * bio_integrity_clone - Callback for cloning bios with integrity metadata
  * @bio:	New bio
  * @bio_src:	Original bio
+ * @gfp_mask:	Memory allocation mask
  * @bs:		bio_set to allocate bip from
  *
  * Description:	Called to allocate a bip when cloning a bio
  */
 int bio_integrity_clone(struct bio *bio, struct bio *bio_src,
-			struct bio_set *bs)
+			gfp_t gfp_mask, struct bio_set *bs)
 {
 	struct bio_integrity_payload *bip_src = bio_src->bi_integrity;
 	struct bio_integrity_payload *bip;
 
 	BUG_ON(bip_src == NULL);
 
-	bip = bio_integrity_alloc_bioset(bio, GFP_NOIO, bip_src->bip_vcnt, bs);
+	bip = bio_integrity_alloc_bioset(bio, gfp_mask, bip_src->bip_vcnt, bs);
 
 	if (bip == NULL)
 		return -EIO;
include/linux/bio.h +2 -2
Index: linux-2.6.28-rc2/include/linux/bio.h
===================================================================
@@ -496,7 +496,7 @@ extern void bio_integrity_endio(struct b
 extern void bio_integrity_advance(struct bio *, unsigned int);
 extern void bio_integrity_trim(struct bio *, unsigned int, unsigned int);
 extern void bio_integrity_split(struct bio *, struct bio_pair *, int);
-extern int bio_integrity_clone(struct bio *, struct bio *, struct bio_set *);
+extern int bio_integrity_clone(struct bio *, struct bio *, gfp_t, struct bio_set *);
 extern int bioset_integrity_create(struct bio_set *, int);
 extern void bioset_integrity_free(struct bio_set *);
 extern void bio_integrity_init_slab(void);

@@ -507,7 +507,7 @@ extern void bio_integrity_init_slab(void
 #define bioset_integrity_create(a, b)	(0)
 #define bio_integrity_prep(a)		(0)
 #define bio_integrity_enabled(a)	(0)
-#define bio_integrity_clone(a, b, c)	(0)
+#define bio_integrity_clone(a, b, c, d)	(0)
 #define bioset_integrity_free(a)	do { } while (0)
 #define bio_integrity_free(a, b)	do { } while (0)
 #define bio_integrity_endio(a, b)	do { } while (0)
fs/bio.c +1 -1
Index: linux-2.6.28-rc2/fs/bio.c
===================================================================
@@ -309,7 +309,7 @@ struct bio *bio_clone(struct bio *bio, g
 	if (bio_integrity(bio)) {
 		int ret;
 
-		ret = bio_integrity_clone(b, bio, fs_bio_set);
+		ret = bio_integrity_clone(b, bio, gfp_mask, fs_bio_set);
 
 		if (ret < 0)
 			return NULL;
Acked-by on v1 1/1
#1 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Jun'ichi" == Jun'ichi Nomura <[email protected]> writes:

Jun'ichi> What do you think about adding gfp_mask parameter to Jun'ichi> bio_integrity_clone() like the attached patch?

Jun'ichi> It also seems natural to inherit gfp_mask from bio_clone Jun'ichi> when bio_integrity_clone is called from it. Since Jun'ichi> bio_clone() is the only caller of bio_integrity_clone() in Jun'ichi> 2.6.28-rc2 and all in-tree callers of bio_clone() are using Jun'ichi> GFP_NOIO, the patch does nothing other than changing the Jun'ichi> interface.

Looks good to me. I'll update the non-multipath DM bits accordingly.

Acked-by Martin K. Petersen <[email protected]>
Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
#2 lore raw
Quoted 12 lines · Martin K. Petersen
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Jun'ichi" == Jun'ichi Nomura <[email protected]> writes:

Jun'ichi> What do you think about adding gfp_mask parameter to Jun'ichi> bio_integrity_clone() like the attached patch?

Jun'ichi> It also seems natural to inherit gfp_mask from bio_clone Jun'ichi> when bio_integrity_clone is called from it. Since Jun'ichi> bio_clone() is the only caller of bio_integrity_clone() in Jun'ichi> 2.6.28-rc2 and all in-tree callers of bio_clone() are using Jun'ichi> GFP_NOIO, the patch does nothing other than changing the Jun'ichi> interface.

Quoted 3 lines

Looks good to me. I'll update the non-multipath DM bits accordingly.

Acked-by Martin K. Petersen <[email protected]>

Shall I just include this in the 2.6.29 branch?

Signature · 1 line
Jens Axboe
#3 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Jens" == Jens Axboe <[email protected]> writes:

Jens> Shall I just include this in the 2.6.29 branch?

Sure!

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering