diff mbox

[1/1] mm/block: convert rw_page users to bio op use

Message ID 38523086-1436-fdbf-bd89-9f31ad134bed@kernel.dk (mailing list archive)
State New, archived
Headers show

Commit Message

Jens Axboe Aug. 4, 2016, 7:27 p.m. UTC
On 08/04/2016 11:50 AM, kbuild test robot wrote:
> Hi Mike,
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on next-20160804]
> [cannot apply to v4.7]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
> config: i386-tinyconfig (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>    mm/filemap.c: In function 'page_endio':
>>> mm/filemap.c:892:7: error: implicit declaration of function 'op_is_write' [-Werror=implicit-function-declaration]
>      if (!op_is_write(op)) {
>           ^~~~~~~~~~~

Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
in fs.h, and making the REQ_OP_* enum generally available as well? That
should fix it.


   */

Comments

Mike Christie Aug. 4, 2016, 7:59 p.m. UTC | #1
On 08/04/2016 02:27 PM, Jens Axboe wrote:
> On 08/04/2016 11:50 AM, kbuild test robot wrote:
>> Hi Mike,
>>
>> [auto build test ERROR on linus/master]
>> [also build test ERROR on next-20160804]
>> [cannot apply to v4.7]
>> [if your patch is applied to the wrong git tree, please drop us a note
>> to help improve the system]
>>
>> url:   
>> https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
>>
>> config: i386-tinyconfig (attached as .config)
>> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=i386
>>
>> All errors (new ones prefixed by >>):
>>
>>    mm/filemap.c: In function 'page_endio':
>>>> mm/filemap.c:892:7: error: implicit declaration of function
>>>> 'op_is_write' [-Werror=implicit-function-declaration]
>>      if (!op_is_write(op)) {
>>           ^~~~~~~~~~~
> 
> Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
> in fs.h, and making the REQ_OP_* enum generally available as well? That
> should fix it.
> 
> 
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index f254eb264924..14b28ff2caf8 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -18,6 +18,17 @@ struct cgroup_subsys_state;
>  typedef void (bio_end_io_t) (struct bio *);
>  typedef void (bio_destructor_t) (struct bio *);
> 
> +enum req_op {
> +    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 */
> +};
> +
> +#define REQ_OP_BITS 3
> +
>  #ifdef CONFIG_BLOCK
>  /*
>   * main unit of I/O for the block layer and lower layers (ie drivers and
> @@ -228,17 +239,6 @@ enum rq_flag_bits {
>  #define REQ_HASHED        (1ULL << __REQ_HASHED)
>  #define REQ_MQ_INFLIGHT        (1ULL << __REQ_MQ_INFLIGHT)
> 
> -enum req_op {
> -    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 */
> -};
> -
> -#define REQ_OP_BITS 3
> -
>  typedef unsigned int blk_qc_t;
>  #define BLK_QC_T_NONE    -1U
>  #define BLK_QC_T_SHIFT    16
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f2a69f20926f..13cc1bfce9aa 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2465,12 +2465,13 @@ extern void init_special_inode(struct inode *,
> umode_t, dev_t);
>  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;
>  }
> 
> +#ifdef CONFIG_BLOCK
> +
>  /*
>   * return data direction, READ or WRITE
>   */
> 


Looks ok to me. Did a quick build and test run with it.


--
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  http://vger.kernel.org/majordomo-info.html
Jens Axboe Aug. 4, 2016, 8:15 p.m. UTC | #2
On 08/04/2016 01:59 PM, Mike Christie wrote:
> On 08/04/2016 02:27 PM, Jens Axboe wrote:
>> On 08/04/2016 11:50 AM, kbuild test robot wrote:
>>> Hi Mike,
>>>
>>> [auto build test ERROR on linus/master]
>>> [also build test ERROR on next-20160804]
>>> [cannot apply to v4.7]
>>> [if your patch is applied to the wrong git tree, please drop us a note
>>> to help improve the system]
>>>
>>> url:
>>> https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
>>>
>>> config: i386-tinyconfig (attached as .config)
>>> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
>>> reproduce:
>>>         # save the attached .config to linux build tree
>>>         make ARCH=i386
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>    mm/filemap.c: In function 'page_endio':
>>>>> mm/filemap.c:892:7: error: implicit declaration of function
>>>>> 'op_is_write' [-Werror=implicit-function-declaration]
>>>      if (!op_is_write(op)) {
>>>           ^~~~~~~~~~~
>>
>> Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
>> in fs.h, and making the REQ_OP_* enum generally available as well? That
>> should fix it.
>>
>>
>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>> index f254eb264924..14b28ff2caf8 100644
>> --- a/include/linux/blk_types.h
>> +++ b/include/linux/blk_types.h
>> @@ -18,6 +18,17 @@ struct cgroup_subsys_state;
>>  typedef void (bio_end_io_t) (struct bio *);
>>  typedef void (bio_destructor_t) (struct bio *);
>>
>> +enum req_op {
>> +    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 */
>> +};
>> +
>> +#define REQ_OP_BITS 3
>> +
>>  #ifdef CONFIG_BLOCK
>>  /*
>>   * main unit of I/O for the block layer and lower layers (ie drivers and
>> @@ -228,17 +239,6 @@ enum rq_flag_bits {
>>  #define REQ_HASHED        (1ULL << __REQ_HASHED)
>>  #define REQ_MQ_INFLIGHT        (1ULL << __REQ_MQ_INFLIGHT)
>>
>> -enum req_op {
>> -    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 */
>> -};
>> -
>> -#define REQ_OP_BITS 3
>> -
>>  typedef unsigned int blk_qc_t;
>>  #define BLK_QC_T_NONE    -1U
>>  #define BLK_QC_T_SHIFT    16
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index f2a69f20926f..13cc1bfce9aa 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2465,12 +2465,13 @@ extern void init_special_inode(struct inode *,
>> umode_t, dev_t);
>>  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;
>>  }
>>
>> +#ifdef CONFIG_BLOCK
>> +
>>  /*
>>   * return data direction, READ or WRITE
>>   */
>>
>
>
> Looks ok to me. Did a quick build and test run with it.

Is anyone using the op_flags in the ->rw_page() strategy?
Mike Christie Aug. 4, 2016, 9:24 p.m. UTC | #3
On 08/04/2016 03:15 PM, Jens Axboe wrote:
> On 08/04/2016 01:59 PM, Mike Christie wrote:
>> On 08/04/2016 02:27 PM, Jens Axboe wrote:
>>> On 08/04/2016 11:50 AM, kbuild test robot wrote:
>>>> Hi Mike,
>>>>
>>>> [auto build test ERROR on linus/master]
>>>> [also build test ERROR on next-20160804]
>>>> [cannot apply to v4.7]
>>>> [if your patch is applied to the wrong git tree, please drop us a note
>>>> to help improve the system]
>>>>
>>>> url:
>>>> https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
>>>>
>>>>
>>>> config: i386-tinyconfig (attached as .config)
>>>> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
>>>> reproduce:
>>>>         # save the attached .config to linux build tree
>>>>         make ARCH=i386
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>    mm/filemap.c: In function 'page_endio':
>>>>>> mm/filemap.c:892:7: error: implicit declaration of function
>>>>>> 'op_is_write' [-Werror=implicit-function-declaration]
>>>>      if (!op_is_write(op)) {
>>>>           ^~~~~~~~~~~
>>>
>>> Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
>>> in fs.h, and making the REQ_OP_* enum generally available as well? That
>>> should fix it.
>>>
>>>
>>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>>> index f254eb264924..14b28ff2caf8 100644
>>> --- a/include/linux/blk_types.h
>>> +++ b/include/linux/blk_types.h
>>> @@ -18,6 +18,17 @@ struct cgroup_subsys_state;
>>>  typedef void (bio_end_io_t) (struct bio *);
>>>  typedef void (bio_destructor_t) (struct bio *);
>>>
>>> +enum req_op {
>>> +    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 */
>>> +};
>>> +
>>> +#define REQ_OP_BITS 3
>>> +
>>>  #ifdef CONFIG_BLOCK
>>>  /*
>>>   * main unit of I/O for the block layer and lower layers (ie drivers
>>> and
>>> @@ -228,17 +239,6 @@ enum rq_flag_bits {
>>>  #define REQ_HASHED        (1ULL << __REQ_HASHED)
>>>  #define REQ_MQ_INFLIGHT        (1ULL << __REQ_MQ_INFLIGHT)
>>>
>>> -enum req_op {
>>> -    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 */
>>> -};
>>> -
>>> -#define REQ_OP_BITS 3
>>> -
>>>  typedef unsigned int blk_qc_t;
>>>  #define BLK_QC_T_NONE    -1U
>>>  #define BLK_QC_T_SHIFT    16
>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>> index f2a69f20926f..13cc1bfce9aa 100644
>>> --- a/include/linux/fs.h
>>> +++ b/include/linux/fs.h
>>> @@ -2465,12 +2465,13 @@ extern void init_special_inode(struct inode *,
>>> umode_t, dev_t);
>>>  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;
>>>  }
>>>
>>> +#ifdef CONFIG_BLOCK
>>> +
>>>  /*
>>>   * return data direction, READ or WRITE
>>>   */
>>>
>>
>>
>> Looks ok to me. Did a quick build and test run with it.
> 
> Is anyone using the op_flags in the ->rw_page() strategy?

Not in the current code. I had just passed it in because the code was
currently passing that extra info, and I thought the original
implementer was going to use it in the future.
--
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  http://vger.kernel.org/majordomo-info.html
Jens Axboe Aug. 4, 2016, 9:26 p.m. UTC | #4
On 08/04/2016 03:24 PM, Mike Christie wrote:
> On 08/04/2016 03:15 PM, Jens Axboe wrote:
>> On 08/04/2016 01:59 PM, Mike Christie wrote:
>>> On 08/04/2016 02:27 PM, Jens Axboe wrote:
>>>> On 08/04/2016 11:50 AM, kbuild test robot wrote:
>>>>> Hi Mike,
>>>>>
>>>>> [auto build test ERROR on linus/master]
>>>>> [also build test ERROR on next-20160804]
>>>>> [cannot apply to v4.7]
>>>>> [if your patch is applied to the wrong git tree, please drop us a note
>>>>> to help improve the system]
>>>>>
>>>>> url:
>>>>> https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
>>>>>
>>>>>
>>>>> config: i386-tinyconfig (attached as .config)
>>>>> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
>>>>> reproduce:
>>>>>         # save the attached .config to linux build tree
>>>>>         make ARCH=i386
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>    mm/filemap.c: In function 'page_endio':
>>>>>>> mm/filemap.c:892:7: error: implicit declaration of function
>>>>>>> 'op_is_write' [-Werror=implicit-function-declaration]
>>>>>      if (!op_is_write(op)) {
>>>>>           ^~~~~~~~~~~
>>>>
>>>> Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
>>>> in fs.h, and making the REQ_OP_* enum generally available as well? That
>>>> should fix it.
>>>>
>>>>
>>>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>>>> index f254eb264924..14b28ff2caf8 100644
>>>> --- a/include/linux/blk_types.h
>>>> +++ b/include/linux/blk_types.h
>>>> @@ -18,6 +18,17 @@ struct cgroup_subsys_state;
>>>>  typedef void (bio_end_io_t) (struct bio *);
>>>>  typedef void (bio_destructor_t) (struct bio *);
>>>>
>>>> +enum req_op {
>>>> +    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 */
>>>> +};
>>>> +
>>>> +#define REQ_OP_BITS 3
>>>> +
>>>>  #ifdef CONFIG_BLOCK
>>>>  /*
>>>>   * main unit of I/O for the block layer and lower layers (ie drivers
>>>> and
>>>> @@ -228,17 +239,6 @@ enum rq_flag_bits {
>>>>  #define REQ_HASHED        (1ULL << __REQ_HASHED)
>>>>  #define REQ_MQ_INFLIGHT        (1ULL << __REQ_MQ_INFLIGHT)
>>>>
>>>> -enum req_op {
>>>> -    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 */
>>>> -};
>>>> -
>>>> -#define REQ_OP_BITS 3
>>>> -
>>>>  typedef unsigned int blk_qc_t;
>>>>  #define BLK_QC_T_NONE    -1U
>>>>  #define BLK_QC_T_SHIFT    16
>>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>>> index f2a69f20926f..13cc1bfce9aa 100644
>>>> --- a/include/linux/fs.h
>>>> +++ b/include/linux/fs.h
>>>> @@ -2465,12 +2465,13 @@ extern void init_special_inode(struct inode *,
>>>> umode_t, dev_t);
>>>>  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;
>>>>  }
>>>>
>>>> +#ifdef CONFIG_BLOCK
>>>> +
>>>>  /*
>>>>   * return data direction, READ or WRITE
>>>>   */
>>>>
>>>
>>>
>>> Looks ok to me. Did a quick build and test run with it.
>>
>> Is anyone using the op_flags in the ->rw_page() strategy?
>
> Not in the current code. I had just passed it in because the code was
> currently passing that extra info, and I thought the original
> implementer was going to use it in the future.

I killed it, we can always add it if we need it. But most likely it'll
just linger, so might as well just kill it off.

This is what I ended up with:

http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus
Dave Chinner Aug. 5, 2016, 2:18 a.m. UTC | #5
On Thu, Aug 04, 2016 at 01:27:37PM -0600, Jens Axboe wrote:
> On 08/04/2016 11:50 AM, kbuild test robot wrote:
> >Hi Mike,
> >
> >[auto build test ERROR on linus/master]
> >[also build test ERROR on next-20160804]
> >[cannot apply to v4.7]
> >[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> >url:    https://github.com/0day-ci/linux/commits/Mike-Christie/mm-block-convert-rw_page-users-to-bio-op-use/20160805-012041
> >config: i386-tinyconfig (attached as .config)
> >compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> >reproduce:
> >        # save the attached .config to linux build tree
> >        make ARCH=i386
> >
> >All errors (new ones prefixed by >>):
> >
> >   mm/filemap.c: In function 'page_endio':
> >>>mm/filemap.c:892:7: error: implicit declaration of function 'op_is_write' [-Werror=implicit-function-declaration]
> >     if (!op_is_write(op)) {
> >          ^~~~~~~~~~~
> 
> Mike, how about moving op_is_write() outside of CONFIG_BLOCK protection
> in fs.h, and making the REQ_OP_* enum generally available as well? That
> should fix it.

Give this is being spread all over the kernel way outside the block
layer and IO path, shouldn't this have some kind of namespace
component to the name? i.e "req_op_is_write()"?

Cheers,

Dave.
Christoph Hellwig Aug. 5, 2016, 7:53 a.m. UTC | #6
On Fri, Aug 05, 2016 at 12:18:55PM +1000, Dave Chinner wrote:
> Give this is being spread all over the kernel way outside the block
> layer and IO path, shouldn't this have some kind of namespace
> component to the name? i.e "req_op_is_write()"?

The READ/WRITE #defines alias to the request ops, so this helper is
rather global as well.  Not that I like it, but it will take a while
to clean that up, including making various things __bitwise annotated
enums to get proper sparse type checking.
--
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  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index f254eb264924..14b28ff2caf8 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -18,6 +18,17 @@  struct cgroup_subsys_state;
  typedef void (bio_end_io_t) (struct bio *);
  typedef void (bio_destructor_t) (struct bio *);

+enum req_op {
+	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 */
+};
+
+#define REQ_OP_BITS 3
+
  #ifdef CONFIG_BLOCK
  /*
   * main unit of I/O for the block layer and lower layers (ie drivers and
@@ -228,17 +239,6 @@  enum rq_flag_bits {
  #define REQ_HASHED		(1ULL << __REQ_HASHED)
  #define REQ_MQ_INFLIGHT		(1ULL << __REQ_MQ_INFLIGHT)

-enum req_op {
-	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 */
-};
-
-#define REQ_OP_BITS 3
-
  typedef unsigned int blk_qc_t;
  #define BLK_QC_T_NONE	-1U
  #define BLK_QC_T_SHIFT	16
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f2a69f20926f..13cc1bfce9aa 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2465,12 +2465,13 @@  extern void init_special_inode(struct inode *, 
umode_t, dev_t);
  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;
  }

+#ifdef CONFIG_BLOCK
+
  /*
   * return data direction, READ or WRITE