Re: [PATCH 7/7] block: add a proper block layer data direction encoding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 20, 2016 at 8:12 AM, Christoph Hellwig <hch@xxxxxx> 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@xxxxxx>
> ---
>  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@xxxxxxxxxxx>

> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> 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=



-- 
Shaun Tancheff
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux