block: Use REQ_OP_WRITE instead of its integer constant 1 in op_is_write()
PATCH 1/1 block: Use REQ_OP_WRITE instead of its integer constant 1 in op_is_write()
From: Bean Huo <[email protected]>
Use the enums REQ_OP_WRITE in op_is_write() to make it less maintenance requirement and more readable
--- include/linux/blk_types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/blk_types.h +1 -1
@@ -463,7 +463,7 @@ static inline void bio_set_op_attrs(struct bio *bio, unsigned op,static inline bool op_is_write (unsigned int op) { -return (op &1 ); +return (op &REQ_OP_WRITE ); }/*
Signature · 1 line
2.25.1
Quoted 15 lines · Bean Huo
From: Bean Huo <[email protected]>
Use the enums
REQ_OP_WRITEinop_is_write()to make it less maintenance requirement and more readableSigned-off-by Bean Huo <[email protected]>--- include/linux/blk_types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/blk_types.h +0
-0
include/linux/blk_types.h static inline void bio_set_op_attrs(struct bio *bio, unsigned op, · line 467
Quoted 6 lines
include/linux/blk_types.h +1
-1@@ -463,7 +463,7 @@ static inline void bio_set_op_attrs(struct bio *bio, unsigned op,static inline bool op_is_write (unsigned int op) { -return (op &1 ); +return (op &REQ_OP_WRITE ); }
See the comment for "enum req_opf":
/*
* 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 no
* meaning.
*/
So using "1" is correct. Using REQ_OP_WRITE is confusing as it seem to imply that op_is_write() tests for "op is REQ_OP_WRITE" instead of the intended "op is transferring data TO the device". If anything, op_is_write() could be renamed to clarify that.
Signature · 2 lines
Damien Le Moal Western Digital Research
Quoted 39 lines · Damien Le Moal
Quoted 15 lines · Bean Huo
From: Bean Huo <[email protected]>
Use the enums
REQ_OP_WRITEinop_is_write()to make it less maintenance requirement and more readableSigned-off-by Bean Huo <[email protected]>--- include/linux/blk_types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/blk_types.h +0
-0include/linux/blk_types.h static inline void bio_set_op_attrs(struct bio *bio, unsigned op, · line 467
Quoted 6 lines
include/linux/blk_types.h +1
-1@@ -463,7 +463,7 @@ static inline void bio_set_op_attrs(struct bio *bio, unsigned op,static inline bool op_is_write (unsigned int op) { -return (op &1 ); +return (op &REQ_OP_WRITE ); }See the comment for "enum
req_opf":/*
* 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 no
* meaning.
*/
Quoted 4 lines
So using "1" is correct. Using
REQ_OP_WRITEis confusing as it seem to imply thatop_is_write()tests for "op isREQ_OP_WRITE" instead of the intended "op is transferring data TO the device". If anything,op_is_write()could be renamed to clarify that.
Yeah, REQ_OP_WRITE is a value, not a flag. The op_is_write() tests the data direction flag, which coincidentally happens to be the same as the current, somewhat arbitrarily chosen value of REQ_OP_WRITE. The op could just as easily have been set to 0xff.
Bean,
include/linux/blk_types.h static inline void bio_set_op_attrs(struct bio *bio, unsigned op, · line 467
Quoted 5 lines
static inline bool
op_is_write(unsigned int op)
{include/linux/blk_types.h +1
-1-return (op &1 ); +return (op &REQ_OP_WRITE ); }
As pointed out by Damien it supposed to indicate data out and not only write request REQ_OP_WRITE.
Quoted 5 lines
/*
--
2.25.1