On 2021/11/16 6:58, Bean Huo wrote:
> From: Bean Huo <beanhuo@xxxxxxxxxx>
>
> Use the enums REQ_OP_WRITE in op_is_write() to make it less maintenance
> requirement and more readable
>
> Signed-off-by: Bean Huo <beanhuo@xxxxxxxxxx>
> ---
> 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 fe065c394fff..5b5924a7e754 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -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.
--
Damien Le Moal
Western Digital Research
