All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] block: Improve bio_set_op_attrs() robustness
@ 2016-09-14  8:42 ` Bart Van Assche
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-09-14  8:42 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

Hi Jens,

bio_set_op_attrs() does not yet check whether the "op_flags" field 
overflows into the "op" field. Since adding support for SMR requires to 
introduce more REQ_* flags I think it is important to have such a check. 
Hence this patch series. Please consider these patches for inclusion in 
the upstream Linux kernel.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH 0/3] block: Improve bio_set_op_attrs() robustness
@ 2016-09-14  8:42 ` Bart Van Assche
  0 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-09-14  8:42 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

Hi Jens,

bio_set_op_attrs() does not yet check whether the "op_flags" field 
overflows into the "op" field. Since adding support for SMR requires to 
introduce more REQ_* flags I think it is important to have such a check. 
Hence this patch series. Please consider these patches for inclusion in 
the upstream Linux kernel.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf
  2016-09-14  8:42 ` Bart Van Assche
  (?)
@ 2016-09-14  8:44 ` Bart Van Assche
  2016-09-14  8:52     ` Johannes Thumshirn
  2016-09-14  9:56   ` Christoph Hellwig
  -1 siblings, 2 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-09-14  8:44 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

Make it clear that the sizeof(unsigned int) expression in BIO_OP_SHIFT
refers to the bi_opf member of struct bio.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Damien Le Moal <damien.lemoal@hgst.com>
---
 include/linux/blk_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 436f43f..1e1ef21 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -89,7 +89,7 @@ struct bio {
 	struct bio_vec		bi_inline_vecs[0];
 };
 
-#define BIO_OP_SHIFT	(8 * sizeof(unsigned int) - REQ_OP_BITS)
+#define BIO_OP_SHIFT	(8 * FIELD_SIZEOF(struct bio, bi_opf) - REQ_OP_BITS)
 #define bio_op(bio)	((bio)->bi_opf >> BIO_OP_SHIFT)
 
 #define bio_set_op_attrs(bio, op, op_flags) do {		\
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags()
  2016-09-14  8:42 ` Bart Van Assche
  (?)
  (?)
@ 2016-09-14  8:45 ` Bart Van Assche
  2016-09-14  8:53     ` Johannes Thumshirn
                     ` (2 more replies)
  -1 siblings, 3 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-09-14  8:45 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

Introduce the bio_flags() macro. Ensure that the second argument of
bio_set_op_attrs() only contains flags and no operation. This patch
does not change any functionality.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Chris Mason <clm@fb.com> (maintainer:BTRFS FILE SYSTEM)
Cc: Josef Bacik <jbacik@fb.com> (maintainer:BTRFS FILE SYSTEM)
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Damien Le Moal <damien.lemoal@hgst.com>
---
 drivers/md/dm-crypt.c     | 2 +-
 fs/btrfs/inode.c          | 5 +++--
 include/linux/blk_types.h | 3 ++-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8742957..0448e7e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1136,7 +1136,7 @@ static void clone_init(struct dm_crypt_io *io, struct bio *clone)
 	clone->bi_private = io;
 	clone->bi_end_io  = crypt_endio;
 	clone->bi_bdev    = cc->dev->bdev;
-	bio_set_op_attrs(clone, bio_op(io->base_bio), io->base_bio->bi_opf);
+	bio_set_op_attrs(clone, bio_op(io->base_bio), bio_flags(io->base_bio));
 }
 
 static int kcryptd_io_read(struct dm_crypt_io *io, gfp_t gfp)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e6811c4..ca01106 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8412,7 +8412,7 @@ static int btrfs_submit_direct_hook(struct btrfs_dio_private *dip,
 	if (!bio)
 		return -ENOMEM;
 
-	bio_set_op_attrs(bio, bio_op(orig_bio), orig_bio->bi_opf);
+	bio_set_op_attrs(bio, bio_op(orig_bio), bio_flags(orig_bio));
 	bio->bi_private = dip;
 	bio->bi_end_io = btrfs_end_dio_bio;
 	btrfs_io_bio(bio)->logical = file_offset;
@@ -8450,7 +8450,8 @@ next_block:
 						  start_sector, GFP_NOFS);
 			if (!bio)
 				goto out_err;
-			bio_set_op_attrs(bio, bio_op(orig_bio), orig_bio->bi_opf);
+			bio_set_op_attrs(bio, bio_op(orig_bio),
+					 bio_flags(orig_bio));
 			bio->bi_private = dip;
 			bio->bi_end_io = btrfs_end_dio_bio;
 			btrfs_io_bio(bio)->logical = file_offset;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 1e1ef21..311fa2f 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -90,11 +90,12 @@ struct bio {
 };
 
 #define BIO_OP_SHIFT	(8 * FIELD_SIZEOF(struct bio, bi_opf) - REQ_OP_BITS)
+#define bio_flags(bio)	((bio)->bi_opf & ((1 << BIO_OP_SHIFT) - 1))
 #define bio_op(bio)	((bio)->bi_opf >> BIO_OP_SHIFT)
 
 #define bio_set_op_attrs(bio, op, op_flags) do {		\
 	WARN_ON(op >= (1 << REQ_OP_BITS));			\
-	(bio)->bi_opf &= ((1 << BIO_OP_SHIFT) - 1);		\
+	(bio)->bi_opf = bio_flags(bio);				\
 	(bio)->bi_opf |= ((unsigned int) (op) << BIO_OP_SHIFT);	\
 	(bio)->bi_opf |= op_flags;				\
 } while (0)
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* [PATCH 3/3] block: Improve bio_set_op_attrs() robustness
  2016-09-14  8:42 ` Bart Van Assche
                   ` (2 preceding siblings ...)
  (?)
@ 2016-09-14  8:46 ` Bart Van Assche
  2016-09-14  8:55     ` Johannes Thumshirn
  2016-09-14  9:59   ` Christoph Hellwig
  -1 siblings, 2 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-09-14  8:46 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

Since REQ_OP_BITS == 3 and __REQ_NR_BITS == 30 it is not that hard
to pass an op_flags argument to bio_set_op_attrs() that is larger
than the number of bits reserved for the op_flags argument. Complain
if this happens. Additionally, ensure that negative arguments trigger
a complaint (1 << ... is signed while 1U << ... is unsigned; adding
0U to an integer expression causes it to be promoted to an unsigned
type).

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Damien Le Moal <damien.lemoal@hgst.com>
---
 include/linux/blk_types.h | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 311fa2f..53ee1a2 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -93,11 +93,18 @@ struct bio {
 #define bio_flags(bio)	((bio)->bi_opf & ((1 << BIO_OP_SHIFT) - 1))
 #define bio_op(bio)	((bio)->bi_opf >> BIO_OP_SHIFT)
 
-#define bio_set_op_attrs(bio, op, op_flags) do {		\
-	WARN_ON(op >= (1 << REQ_OP_BITS));			\
-	(bio)->bi_opf = bio_flags(bio);				\
-	(bio)->bi_opf |= ((unsigned int) (op) << BIO_OP_SHIFT);	\
-	(bio)->bi_opf |= op_flags;				\
+#define bio_set_op_attrs(bio, op, op_flags) do {			\
+	if (__builtin_constant_p(op))					\
+		BUILD_BUG_ON((op) + 0U >= (1U << REQ_OP_BITS));		\
+	else								\
+		WARN_ON_ONCE((op) + 0U >= (1U << REQ_OP_BITS));		\
+	if (__builtin_constant_p(op_flags))				\
+		BUILD_BUG_ON((op_flags) + 0U >= (1U << BIO_OP_SHIFT));	\
+	else								\
+		WARN_ON_ONCE((op_flags) + 0U >= (1U << BIO_OP_SHIFT));	\
+	(bio)->bi_opf = bio_flags(bio);					\
+	(bio)->bi_opf |= (((op) + 0U) << BIO_OP_SHIFT);			\
+	(bio)->bi_opf |= (op_flags);					\
 } while (0)
 
 #define BIO_RESET_BYTES		offsetof(struct bio, bi_max_vecs)
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf
  2016-09-14  8:44 ` [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf Bart Van Assche
@ 2016-09-14  8:52     ` Johannes Thumshirn
  2016-09-14  9:56   ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:52 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:44:12AM +0200, Bart Van Assche wrote:
> Make it clear that the sizeof(unsigned int) expression in BIO_OP_SHIFT
> refers to the bi_opf member of struct bio.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf
@ 2016-09-14  8:52     ` Johannes Thumshirn
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:52 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:44:12AM +0200, Bart Van Assche wrote:
> Make it clear that the sizeof(unsigned int) expression in BIO_OP_SHIFT
> refers to the bi_opf member of struct bio.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags()
  2016-09-14  8:45 ` [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags() Bart Van Assche
@ 2016-09-14  8:53     ` Johannes Thumshirn
  2016-09-14  9:58   ` Christoph Hellwig
  2016-09-14  9:59   ` Christoph Hellwig
  2 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:53 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:45:36AM +0200, Bart Van Assche wrote:
> Introduce the bio_flags() macro. Ensure that the second argument of
> bio_set_op_attrs() only contains flags and no operation. This patch
> does not change any functionality.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Chris Mason <clm@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Josef Bacik <jbacik@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags()
@ 2016-09-14  8:53     ` Johannes Thumshirn
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:53 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:45:36AM +0200, Bart Van Assche wrote:
> Introduce the bio_flags() macro. Ensure that the second argument of
> bio_set_op_attrs() only contains flags and no operation. This patch
> does not change any functionality.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Chris Mason <clm@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Josef Bacik <jbacik@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] block: Improve bio_set_op_attrs() robustness
  2016-09-14  8:46 ` [PATCH 3/3] block: Improve bio_set_op_attrs() robustness Bart Van Assche
@ 2016-09-14  8:55     ` Johannes Thumshirn
  2016-09-14  9:59   ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:55 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:46:22AM +0200, Bart Van Assche wrote:
> Since REQ_OP_BITS == 3 and __REQ_NR_BITS == 30 it is not that hard
> to pass an op_flags argument to bio_set_op_attrs() that is larger
> than the number of bits reserved for the op_flags argument. Complain
> if this happens. Additionally, ensure that negative arguments trigger
> a complaint (1 << ... is signed while 1U << ... is unsigned; adding
> 0U to an integer expression causes it to be promoted to an unsigned
> type).
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] block: Improve bio_set_op_attrs() robustness
@ 2016-09-14  8:55     ` Johannes Thumshirn
  0 siblings, 0 replies; 17+ messages in thread
From: Johannes Thumshirn @ 2016-09-14  8:55 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:46:22AM +0200, Bart Van Assche wrote:
> Since REQ_OP_BITS == 3 and __REQ_NR_BITS == 30 it is not that hard
> to pass an op_flags argument to bio_set_op_attrs() that is larger
> than the number of bits reserved for the op_flags argument. Complain
> if this happens. Additionally, ensure that negative arguments trigger
> a complaint (1 << ... is signed while 1U << ... is unsigned; adding
> 0U to an integer expression causes it to be promoted to an unsigned
> type).
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf
  2016-09-14  8:44 ` [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf Bart Van Assche
  2016-09-14  8:52     ` Johannes Thumshirn
@ 2016-09-14  9:56   ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2016-09-14  9:56 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags()
  2016-09-14  8:45 ` [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags() Bart Van Assche
  2016-09-14  8:53     ` Johannes Thumshirn
@ 2016-09-14  9:58   ` Christoph Hellwig
  2016-09-14  9:59   ` Christoph Hellwig
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2016-09-14  9:58 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:45:36AM +0200, Bart Van Assche wrote:
> Introduce the bio_flags() macro. Ensure that the second argument of
> bio_set_op_attrs() only contains flags and no operation. This patch
> does not change any functionality.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Christie <mchristi@redhat.com>
> Cc: Chris Mason <clm@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Josef Bacik <jbacik@fb.com> (maintainer:BTRFS FILE SYSTEM)
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Damien Le Moal <damien.lemoal@hgst.com>
> ---
>  drivers/md/dm-crypt.c     | 2 +-
>  fs/btrfs/inode.c          | 5 +++--
>  include/linux/blk_types.h | 3 ++-
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 8742957..0448e7e 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1136,7 +1136,7 @@ static void clone_init(struct dm_crypt_io *io, struct bio *clone)
>  	clone->bi_private = io;
>  	clone->bi_end_io  = crypt_endio;
>  	clone->bi_bdev    = cc->dev->bdev;
> -	bio_set_op_attrs(clone, bio_op(io->base_bio), io->base_bio->bi_opf);
> +	bio_set_op_attrs(clone, bio_op(io->base_bio), bio_flags(io->base_bio));

Given that bio_set_op_attrs calls bio_flags internall do we need
the call here as well?

The other option might be to check that we only get flags inside the
bio_flags space and let the caller sort it out, which sounds useful
to me.

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] block: Improve bio_set_op_attrs() robustness
  2016-09-14  8:46 ` [PATCH 3/3] block: Improve bio_set_op_attrs() robustness Bart Van Assche
  2016-09-14  8:55     ` Johannes Thumshirn
@ 2016-09-14  9:59   ` Christoph Hellwig
  1 sibling, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2016-09-14  9:59 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

On Wed, Sep 14, 2016 at 10:46:22AM +0200, Bart Van Assche wrote:
> Since REQ_OP_BITS == 3 and __REQ_NR_BITS == 30 it is not that hard
> to pass an op_flags argument to bio_set_op_attrs() that is larger
> than the number of bits reserved for the op_flags argument. Complain
> if this happens. Additionally, ensure that negative arguments trigger
> a complaint (1 << ... is signed while 1U << ... is unsigned; adding
> 0U to an integer expression causes it to be promoted to an unsigned
> type).

And this does the proper check.

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags()
  2016-09-14  8:45 ` [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags() Bart Van Assche
  2016-09-14  8:53     ` Johannes Thumshirn
  2016-09-14  9:58   ` Christoph Hellwig
@ 2016-09-14  9:59   ` Christoph Hellwig
  2 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2016-09-14  9:59 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jens Axboe, Mike Christie, Chris Mason, Josef Bacik,
	Mike Snitzer, Christoph Hellwig, Hannes Reinecke, Damien Le Moal,
	linux-block, linux-btrfs, device-mapper development

Ok, looks fine after reading the next patch:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] block: Improve bio_set_op_attrs() robustness
  2016-09-14  8:42 ` Bart Van Assche
@ 2016-09-14 14:48   ` Jens Axboe
  -1 siblings, 0 replies; 17+ messages in thread
From: Jens Axboe @ 2016-09-14 14:48 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

On 09/14/2016 02:42 AM, Bart Van Assche wrote:
> Hi Jens,
>
> bio_set_op_attrs() does not yet check whether the "op_flags" field
> overflows into the "op" field. Since adding support for SMR requires to
> introduce more REQ_* flags I think it is important to have such a check.
> Hence this patch series. Please consider these patches for inclusion in
> the upstream Linux kernel.

Thanks Bart, I like this series. Applied for 4.9.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] block: Improve bio_set_op_attrs() robustness
@ 2016-09-14 14:48   ` Jens Axboe
  0 siblings, 0 replies; 17+ messages in thread
From: Jens Axboe @ 2016-09-14 14:48 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Mike Christie, Chris Mason, Josef Bacik, Mike Snitzer,
	Christoph Hellwig, Hannes Reinecke, Damien Le Moal, linux-block,
	linux-btrfs, device-mapper development

On 09/14/2016 02:42 AM, Bart Van Assche wrote:
> Hi Jens,
>
> bio_set_op_attrs() does not yet check whether the "op_flags" field
> overflows into the "op" field. Since adding support for SMR requires to
> introduce more REQ_* flags I think it is important to have such a check.
> Hence this patch series. Please consider these patches for inclusion in
> the upstream Linux kernel.

Thanks Bart, I like this series. Applied for 4.9.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-09-14 14:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14  8:42 [PATCH 0/3] block: Improve bio_set_op_attrs() robustness Bart Van Assche
2016-09-14  8:42 ` Bart Van Assche
2016-09-14  8:44 ` [PATCH 1/3] block: Document that bio_op() uses the data type of bio.bi_opf Bart Van Assche
2016-09-14  8:52   ` Johannes Thumshirn
2016-09-14  8:52     ` Johannes Thumshirn
2016-09-14  9:56   ` Christoph Hellwig
2016-09-14  8:45 ` [PATCH 2/3] block, dm-crypt, btrfs: Introduce bio_flags() Bart Van Assche
2016-09-14  8:53   ` Johannes Thumshirn
2016-09-14  8:53     ` Johannes Thumshirn
2016-09-14  9:58   ` Christoph Hellwig
2016-09-14  9:59   ` Christoph Hellwig
2016-09-14  8:46 ` [PATCH 3/3] block: Improve bio_set_op_attrs() robustness Bart Van Assche
2016-09-14  8:55   ` Johannes Thumshirn
2016-09-14  8:55     ` Johannes Thumshirn
2016-09-14  9:59   ` Christoph Hellwig
2016-09-14 14:48 ` [PATCH 0/3] " Jens Axboe
2016-09-14 14:48   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.