| Message ID | 1476969135-32732-8-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 20, 2016 at 8:12 AM, Christoph Hellwig <hch@lst.de> wrote: > Currently the block layer op_is_write, bio_data_dir and rq_data_dir > helper treat every operation that is not a READ as a data out operation. > This worked surprisingly long, but the new REQ_OP_ZONE_REPORT operation > actually adds a second operation that reads data from the device. > Surprisingly nothing critical relied on this direction, but this might > be a good opportunity to properly fix this issue up. > > We take a little inspiration and use the least significant bit of the > operation number to encode the data direction, which just requires us > to renumber the operations to fix this scheme. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > include/linux/blk_types.h | 38 ++++++++++++++++++++++++++++++-------- > include/linux/fs.h | 5 ----- > 2 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index dca972d..70d0ef1 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -131,20 +131,37 @@ struct bio { > /* > * Operations and flags common to the bio and request structures. > * We use 8 bits for encoding the operation, and the remaining 24 for flags. > + * > + * The least significant bit of the operation number indicates the data > + * transfer direction: > + * > + * - if the least significant bit is set transfers are TO the device > + * - if the least significant bit is not set transfers are FROM the device > + * > + * If a operation does not transfer data the least significant bit has not > + * meaning. s/not/no/ > */ > #define REQ_OP_BITS 8 > #define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1) > #define REQ_FLAG_BITS 24 > > enum req_opf { > - REQ_OP_READ, > - REQ_OP_WRITE, > - REQ_OP_DISCARD, /* request to discard sectors */ > - REQ_OP_SECURE_ERASE, /* request to securely erase sectors */ > - REQ_OP_WRITE_SAME, /* write same block many times */ > - REQ_OP_FLUSH, /* request for cache flush */ > - REQ_OP_ZONE_REPORT, /* Get zone information */ > - REQ_OP_ZONE_RESET, /* Reset a zone write pointer */ > + /* read sectors from the device */ > + REQ_OP_READ = 0, > + /* write sectors to the device */ > + REQ_OP_WRITE = 1, > + /* flush the volatile write cache */ > + REQ_OP_FLUSH = 2, > + /* discard sectors */ > + REQ_OP_DISCARD = 3, > + /* get zone information */ > + REQ_OP_ZONE_REPORT = 4, > + /* securely erase sectors */ > + REQ_OP_SECURE_ERASE = 5, > + /* seset a zone write pointer */ > + REQ_OP_ZONE_RESET = 6, > + /* write the same sector many times */ > + REQ_OP_WRITE_SAME = 7, > > REQ_OP_LAST, > }; > @@ -194,6 +211,11 @@ enum req_flag_bits { > #define bio_set_op_attrs(bio, op, op_flags) \ > ((bio)->bi_opf |= (op | op_flags)) > > +static inline bool op_is_write(unsigned int op) > +{ > + return (op & 1); > +} > + > static inline bool op_is_sync(unsigned int op) > { > return (op & REQ_OP_MASK) == REQ_OP_READ || (op & REQ_SYNC); > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 16d2b6e..e3e878f 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2499,11 +2499,6 @@ extern void make_bad_inode(struct inode *); > extern bool is_bad_inode(struct inode *); > > #ifdef CONFIG_BLOCK > -static inline bool op_is_write(unsigned int op) > -{ > - return op == REQ_OP_READ ? false : true; > -} > - > /* > * return data direction, READ or WRITE > */ > -- > 2.1.4 Reviewed-by: Shaun Tancheff <shaun.tancheff@seagate.com> > -- > To unsubscribe from this list: send the line "unsubscribe linux-block" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DQIBAg&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ugl8V50qIHLe856QW0qfG3WVYGOrWzA&m=VA90xC85wjuAj4_m5K1z_dyAn_hmiMmQ0OFQUHtUP0s&s=ZNyaS5RTvusRmspt7B38KmDNeX5FrwdKxnXnJ92Itko&e=
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index dca972d..70d0ef1 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -131,20 +131,37 @@ struct bio { /* * Operations and flags common to the bio and request structures. * We use 8 bits for encoding the operation, and the remaining 24 for flags. + * + * The least significant bit of the operation number indicates the data + * transfer direction: + * + * - if the least significant bit is set transfers are TO the device + * - if the least significant bit is not set transfers are FROM the device + * + * If a operation does not transfer data the least significant bit has not + * meaning. */ #define REQ_OP_BITS 8 #define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1) #define REQ_FLAG_BITS 24 enum req_opf { - REQ_OP_READ, - REQ_OP_WRITE, - REQ_OP_DISCARD, /* request to discard sectors */ - REQ_OP_SECURE_ERASE, /* request to securely erase sectors */ - REQ_OP_WRITE_SAME, /* write same block many times */ - REQ_OP_FLUSH, /* request for cache flush */ - REQ_OP_ZONE_REPORT, /* Get zone information */ - REQ_OP_ZONE_RESET, /* Reset a zone write pointer */ + /* read sectors from the device */ + REQ_OP_READ = 0, + /* write sectors to the device */ + REQ_OP_WRITE = 1, + /* flush the volatile write cache */ + REQ_OP_FLUSH = 2, + /* discard sectors */ + REQ_OP_DISCARD = 3, + /* get zone information */ + REQ_OP_ZONE_REPORT = 4, + /* securely erase sectors */ + REQ_OP_SECURE_ERASE = 5, + /* seset a zone write pointer */ + REQ_OP_ZONE_RESET = 6, + /* write the same sector many times */ + REQ_OP_WRITE_SAME = 7, REQ_OP_LAST, }; @@ -194,6 +211,11 @@ enum req_flag_bits { #define bio_set_op_attrs(bio, op, op_flags) \ ((bio)->bi_opf |= (op | op_flags)) +static inline bool op_is_write(unsigned int op) +{ + return (op & 1); +} + static inline bool op_is_sync(unsigned int op) { return (op & REQ_OP_MASK) == REQ_OP_READ || (op & REQ_SYNC); diff --git a/include/linux/fs.h b/include/linux/fs.h index 16d2b6e..e3e878f 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2499,11 +2499,6 @@ extern void make_bad_inode(struct inode *); extern bool is_bad_inode(struct inode *); #ifdef CONFIG_BLOCK -static inline bool op_is_write(unsigned int op) -{ - return op == REQ_OP_READ ? false : true; -} - /* * return data direction, READ or WRITE */
Currently the block layer op_is_write, bio_data_dir and rq_data_dir helper treat every operation that is not a READ as a data out operation. This worked surprisingly long, but the new REQ_OP_ZONE_REPORT operation actually adds a second operation that reads data from the device. Surprisingly nothing critical relied on this direction, but this might be a good opportunity to properly fix this issue up. We take a little inspiration and use the least significant bit of the operation number to encode the data direction, which just requires us to renumber the operations to fix this scheme. Signed-off-by: Christoph Hellwig <hch@lst.de> --- include/linux/blk_types.h | 38 ++++++++++++++++++++++++++++++-------- include/linux/fs.h | 5 ----- 2 files changed, 30 insertions(+), 13 deletions(-)