Skip to contentSkip to conversation

blk: use REQ_OP_WRITE instead of hard code

PATCH In review Hui Su 2 participants 4 messages Oct 19, 2020 Block layer

lore
Share & tools
lore

Hui Su

author
#0 lore raw

PATCH 1/1 blk: use REQ_OP_WRITE instead of hard code

use REQ_OP_WRITE instead of hard code in
op_is_write().

Signed-off-by Hui Su <[email protected]>
---
 include/linux/blk_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/blk_types.h +1 -1
index 7d7c13238fdb..7b9b02378c24 100644
@@ -440,7 +440,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
#1 lore raw
Quoted 13 lines · Hui Su

use REQ_OP_WRITE instead of hard code in
op_is_write().

Signed-off-by Hui Su <[email protected]>
---
 include/linux/blk_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/blk_types.h +0 -0
index 7d7c13238fdb..7b9b02378c24 100644

include/linux/blk_types.h static inline void bio_set_op_attrs(struct bio *bio, unsigned op, · line 443

Quoted 5 lines
include/linux/blk_types.h +1 -1
@@ -440,7 +440,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);

op_is_write() returns true for every req_op that writes to a device (they all have the lowest bit set), while REQ_OP_WRITE means a WRITE. You'll change the semantics with this patch.

author
#2 lore raw

Yeah, you are right, thanks for your explanation<br/><br/>Maybe we should define a MASK to do this?

Quoted 23 lines · Johannes Thumshirn
Quoted 13 lines · Hui Su

use REQ_OP_WRITE instead of hard code in
op_is_write().

Signed-off-by Hui Su <[email protected]>
---
 include/linux/blk_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/blk_types.h +0 -0
index 7d7c13238fdb..7b9b02378c24 100644

include/linux/blk_types.h static inline void bio_set_op_attrs(struct bio *bio, unsigned op, · line 443

Quoted 5 lines
include/linux/blk_types.h +1 -1
@@ -440,7 +440,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);

op_is_write() returns true for every req_op that writes to a device (they all have the lowest bit set), while REQ_OP_WRITE means a WRITE. You'll change the semantics with this patch.

#3 lore raw
Quoted 1 line · 苏辉

Yeah, you are right, thanks for your explanation<br/><br/>Maybe we should define a MASK to do this?

Why? I personally find a '& 1' way more understandable than a REQ_OP_IS_WRITE_MASK or sth like that. The former I can just read, for the latter I would need to look up the definition to be able to understand the code.