| Message ID | 38523086-1436-fdbf-bd89-9f31ad134bed@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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?
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
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
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.
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 --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