This fixes a regression from
4e1b2d52a80d79296a5d899d73249748dea71a53
block, fs, drivers: remove REQ_OP compat defs and related code
The rw_page users were not converted to use bio/req ops. As a result
bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
be sent down as reads.
Signed-off-by: Mike Christie <[email protected]>
---
drivers/block/brd.c | 17 +++++++----------
drivers/block/zram/zram_drv.c | 28 +++++++++++++++-------------
drivers/nvdimm/btt.c | 18 +++++++++---------
drivers/nvdimm/pmem.c | 12 ++++++------
fs/block_dev.c | 8 +++++---
fs/mpage.c | 2 +-
include/linux/blkdev.h | 3 ++-
include/linux/pagemap.h | 2 +-
mm/filemap.c | 6 +++---
9 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3022dad..9fbbeba 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -300,20 +300,20 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
* Process a single bvec of a bio.
*/
static int brd_do_bvec(struct brd_device *brd, struct page *page,
- unsigned int len, unsigned int off, int rw,
+ unsigned int len, unsigned int off, int op,
sector_t sector)
{
void *mem;
int err = 0;
- if (rw != READ) {
+ if (op_is_write(op)) {
err = copy_to_brd_setup(brd, sector, len);
if (err)
goto out;
}
mem = kmap_atomic(page);
- if (rw == READ) {
+ if (!op_is_write(op)) {
copy_from_brd(mem + off, brd, sector, len);
flush_dcache_page(page);
} else {
@@ -330,7 +330,6 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
{
struct block_device *bdev = bio->bi_bdev;
struct brd_device *brd = bdev->bd_disk->private_data;
- int rw;
struct bio_vec bvec;
sector_t sector;
struct bvec_iter iter;
@@ -347,14 +346,12 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
goto out;
}
- rw = bio_data_dir(bio);
-
bio_for_each_segment(bvec, bio, iter) {
unsigned int len = bvec.bv_len;
int err;
err = brd_do_bvec(brd, bvec.bv_page, len,
- bvec.bv_offset, rw, sector);
+ bvec.bv_offset, bio_op(bio), sector);
if (err)
goto io_error;
sector += len >> SECTOR_SHIFT;
@@ -369,11 +366,11 @@ io_error:
}
static int brd_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int rw)
+ struct page *page, int op, int op_flags)
{
struct brd_device *brd = bdev->bd_disk->private_data;
- int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, rw, sector);
- page_endio(page, rw & WRITE, err);
+ int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, op, sector);
+ page_endio(page, op, err);
return err;
}
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 7454cf1..f0e126c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -843,15 +843,15 @@ static void zram_bio_discard(struct zram *zram, u32 index,
}
static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
- int offset, int rw)
+ int offset, int op)
{
unsigned long start_time = jiffies;
int ret;
- generic_start_io_acct(rw, bvec->bv_len >> SECTOR_SHIFT,
+ generic_start_io_acct(op, bvec->bv_len >> SECTOR_SHIFT,
&zram->disk->part0);
- if (rw == READ) {
+ if (!op_is_write(op)) {
atomic64_inc(&zram->stats.num_reads);
ret = zram_bvec_read(zram, bvec, index, offset);
} else {
@@ -859,10 +859,10 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
ret = zram_bvec_write(zram, bvec, index, offset);
}
- generic_end_io_acct(rw, &zram->disk->part0, start_time);
+ generic_end_io_acct(op, &zram->disk->part0, start_time);
if (unlikely(ret)) {
- if (rw == READ)
+ if (!op_is_write(op))
atomic64_inc(&zram->stats.failed_reads);
else
atomic64_inc(&zram->stats.failed_writes);
@@ -873,7 +873,7 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
static void __zram_make_request(struct zram *zram, struct bio *bio)
{
- int offset, rw;
+ int offset;
u32 index;
struct bio_vec bvec;
struct bvec_iter iter;
@@ -888,7 +888,6 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
return;
}
- rw = bio_data_dir(bio);
bio_for_each_segment(bvec, bio, iter) {
int max_transfer_size = PAGE_SIZE - offset;
@@ -903,15 +902,18 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
bv.bv_len = max_transfer_size;
bv.bv_offset = bvec.bv_offset;
- if (zram_bvec_rw(zram, &bv, index, offset, rw) < 0)
+ if (zram_bvec_rw(zram, &bv, index, offset,
+ bio_op(bio)) < 0)
goto out;
bv.bv_len = bvec.bv_len - max_transfer_size;
bv.bv_offset += max_transfer_size;
- if (zram_bvec_rw(zram, &bv, index + 1, 0, rw) < 0)
+ if (zram_bvec_rw(zram, &bv, index + 1, 0,
+ bio_op(bio)) < 0)
goto out;
} else
- if (zram_bvec_rw(zram, &bvec, index, offset, rw) < 0)
+ if (zram_bvec_rw(zram, &bvec, index, offset,
+ bio_op(bio)) < 0)
goto out;
update_position(&index, &offset, &bvec);
@@ -968,7 +970,7 @@ static void zram_slot_free_notify(struct block_device *bdev,
}
static int zram_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int rw)
+ struct page *page, int op, int op_flags)
{
int offset, err = -EIO;
u32 index;
@@ -992,7 +994,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
bv.bv_len = PAGE_SIZE;
bv.bv_offset = 0;
- err = zram_bvec_rw(zram, &bv, index, offset, rw);
+ err = zram_bvec_rw(zram, &bv, index, offset, op);
put_zram:
zram_meta_put(zram);
out:
@@ -1005,7 +1007,7 @@ out:
* (e.g., SetPageError, set_page_dirty and extra works).
*/
if (err == 0)
- page_endio(page, rw, 0);
+ page_endio(page, op, 0);
return err;
}
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 9dce03f..6a6208d 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1133,11 +1133,11 @@ static int btt_write_pg(struct btt *btt, struct bio_integrity_payload *bip,
static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
struct page *page, unsigned int len, unsigned int off,
- int rw, sector_t sector)
+ int op, sector_t sector)
{
int ret;
- if (rw == READ) {
+ if (!op_is_write(op)) {
ret = btt_read_pg(btt, bip, page, off, sector, len);
flush_dcache_page(page);
} else {
@@ -1155,7 +1155,7 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
struct bvec_iter iter;
unsigned long start;
struct bio_vec bvec;
- int err = 0, rw;
+ int err = 0;
bool do_acct;
/*
@@ -1170,7 +1170,6 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
}
do_acct = nd_iostat_start(bio, &start);
- rw = bio_data_dir(bio);
bio_for_each_segment(bvec, bio, iter) {
unsigned int len = bvec.bv_len;
@@ -1181,11 +1180,12 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
BUG_ON(len % btt->sector_size);
err = btt_do_bvec(btt, bip, bvec.bv_page, len, bvec.bv_offset,
- rw, iter.bi_sector);
+ bio_op(bio), iter.bi_sector);
if (err) {
dev_info(&btt->nd_btt->dev,
"io error in %s sector %lld, len %d,\n",
- (rw == READ) ? "READ" : "WRITE",
+ (op_is_write(bio_op(bio))) ? "WRITE" :
+ "READ",
(unsigned long long) iter.bi_sector, len);
bio->bi_error = err;
break;
@@ -1200,12 +1200,12 @@ out:
}
static int btt_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int rw)
+ struct page *page, int op, int op_flags)
{
struct btt *btt = bdev->bd_disk->private_data;
- btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, rw, sector);
- page_endio(page, rw & WRITE, 0);
+ btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, op, sector);
+ page_endio(page, op, 0);
return 0;
}
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index b511099..6a7b97d 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -67,7 +67,7 @@ static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
}
static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
- unsigned int len, unsigned int off, int rw,
+ unsigned int len, unsigned int off, int op,
sector_t sector)
{
int rc = 0;
@@ -79,7 +79,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
bad_pmem = true;
- if (rw == READ) {
+ if (!op_is_write(op)) {
if (unlikely(bad_pmem))
rc = -EIO;
else {
@@ -134,7 +134,7 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
do_acct = nd_iostat_start(bio, &start);
bio_for_each_segment(bvec, bio, iter) {
rc = pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len,
- bvec.bv_offset, bio_data_dir(bio),
+ bvec.bv_offset, bio_op(bio),
iter.bi_sector);
if (rc) {
bio->bi_error = rc;
@@ -152,12 +152,12 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
}
static int pmem_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int rw)
+ struct page *page, int op, int op_flags)
{
struct pmem_device *pmem = bdev->bd_queue->queuedata;
int rc;
- rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, rw, sector);
+ rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, op, sector);
/*
* The ->rw_page interface is subtle and tricky. The core
@@ -166,7 +166,7 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
* caused by double completion.
*/
if (rc == 0)
- page_endio(page, rw & WRITE, 0);
+ page_endio(page, op, 0);
return rc;
}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 2033a3f..7ef1737 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -416,7 +416,8 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
result = blk_queue_enter(bdev->bd_queue, false);
if (result)
return result;
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, READ);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
+ REQ_OP_READ, 0);
blk_queue_exit(bdev->bd_queue);
return result;
}
@@ -445,7 +446,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
struct page *page, struct writeback_control *wbc)
{
int result;
- int rw = (wbc->sync_mode == WB_SYNC_ALL) ? WRITE_SYNC : WRITE;
+ int op_flags = (wbc->sync_mode == WB_SYNC_ALL) ? WRITE_SYNC : 0;
const struct block_device_operations *ops = bdev->bd_disk->fops;
if (!ops->rw_page || bdev_get_integrity(bdev))
@@ -455,7 +456,8 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
return result;
set_page_writeback(page);
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, rw);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
+ REQ_OP_WRITE, op_flags);
if (result)
end_page_writeback(page);
else
diff --git a/fs/mpage.c b/fs/mpage.c
index 2ca1f39..7a09c55 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -50,7 +50,7 @@ static void mpage_end_io(struct bio *bio)
bio_for_each_segment_all(bv, bio, i) {
struct page *page = bv->bv_page;
- page_endio(page, bio_data_dir(bio), bio->bi_error);
+ page_endio(page, bio_op(bio), bio->bi_error);
}
bio_put(bio);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index adf3307..3652408 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1673,7 +1673,8 @@ struct blk_dax_ctl {
struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
void (*release) (struct gendisk *, fmode_t);
- int (*rw_page)(struct block_device *, sector_t, struct page *, int rw);
+ int (*rw_page)(struct block_device *, sector_t, struct page *,
+ int op, int op_flags);
int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
long (*direct_access)(struct block_device *, sector_t, void **, pfn_t *,
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 81363b8..4578637 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -510,7 +510,7 @@ static inline void wait_on_page_writeback(struct page *page)
extern void end_page_writeback(struct page *page);
void wait_for_stable_page(struct page *page);
-void page_endio(struct page *page, int rw, int err);
+void page_endio(struct page *page, int op, int err);
/*
* Add an arbitrary waiter to a page's wait queue
diff --git a/mm/filemap.c b/mm/filemap.c
index 3083ded..daef091 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -887,9 +887,9 @@ EXPORT_SYMBOL(end_page_writeback);
* After completing I/O on a page, call this routine to update the page
* flags appropriately
*/
-void page_endio(struct page *page, int rw, int err)
+void page_endio(struct page *page, int op, int err)
{
- if (rw == READ) {
+ if (!op_is_write(op)) {
if (!err) {
SetPageUptodate(page);
} else {
@@ -897,7 +897,7 @@ void page_endio(struct page *page, int rw, int err)
SetPageError(page);
}
unlock_page(page);
- } else { /* rw == WRITE */
+ } else {
if (err) {
SetPageError(page);
if (page->mapping)
--
2.7.2
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)) {
^~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/op_is_write +892 mm/filemap.c
886 /*
887 * After completing I/O on a page, call this routine to update the page
888 * flags appropriately
889 */
890 void page_endio(struct page *page, int op, int err)
891 {
> 892 if (!op_is_write(op)) {
893 if (!err) {
894 SetPageUptodate(page);
895 } else {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Aug 04, 2016 at 12:17:36PM -0500, Mike Christie wrote:
> This fixes a regression from
> 4e1b2d52a80d79296a5d899d73249748dea71a53
> block, fs, drivers: remove REQ_OP compat defs and related code
>
> The rw_page users were not converted to use bio/req ops. As a result
> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
> be sent down as reads.
>
> Signed-off-by: Mike Christie <[email protected]>
Was the intention for this to fix the xfs.mkfs hang I reported with
generic/361 + XFS?
I'm still able to reproduce that hang with this patch applied to the current
linux/master (c1ece76 (linux/master) Merge tag 'media/v4.8-5' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media)
Thanks,
- Ross
On 08/04/2016 01:46 PM, Ross Zwisler wrote:
> On Thu, Aug 04, 2016 at 12:17:36PM -0500, Mike Christie wrote:
>> This fixes a regression from
>> 4e1b2d52a80d79296a5d899d73249748dea71a53
>> block, fs, drivers: remove REQ_OP compat defs and related code
>>
>> The rw_page users were not converted to use bio/req ops. As a result
>> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
>> be sent down as reads.
>>
>> Signed-off-by: Mike Christie <[email protected]>
>
> Was the intention for this to fix the xfs.mkfs hang I reported with
> generic/361 + XFS?
The hang when also using loop? If so, you also need Christoph's patches:
http://marc.info/?l=linux-fsdevel&m=147032181909530&w=2
http://marc.info/?l=linux-fsdevel&m=147031989408783&w=2
My patch just fixes rw_page users like brd.
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
*/
--
Jens Axboe
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.
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?
--
Jens Axboe
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.
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
--
Jens Axboe
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.
--
Dave Chinner
[email protected]
On 08/04/2016 09:18 PM, Dave Chinner wrote:
> 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()"?
>
I made the patch below to do the suggested rename.
Jens, the patch is made over your for-linus branch. I was not sure if
you wanted it for this kernel or since it is just a rename patch for
4.8.
>From 68bd1b7ebea5c447a4e6e6ac15bb0c4f7783e366 Mon Sep 17 00:00:00 2001
From: Mike Christie <[email protected]>
Date: Thu, 4 Aug 2016 22:04:01 -0500
Subject: [PATCH 1/1] block, drivers, mm: rename op_is_write to req_op_is_write
Rename op_is_write to req_op_is_write to prevent future naming conflicts.
Signed-off-by: Mike Christie <[email protected]>
---
block/blk-core.c | 4 ++--
block/blk-merge.c | 2 +-
drivers/ata/libata-scsi.c | 2 +-
drivers/block/brd.c | 4 ++--
drivers/block/drbd/drbd_req.c | 2 +-
drivers/block/loop.c | 4 ++--
drivers/block/umem.c | 2 +-
drivers/block/zram/zram_drv.c | 4 ++--
drivers/md/bcache/io.c | 2 +-
drivers/md/bcache/request.c | 6 +++---
drivers/md/dm-io.c | 4 ++--
drivers/md/dm-kcopyd.c | 6 +++---
drivers/md/raid5.c | 10 +++++-----
drivers/nvdimm/btt.c | 6 +++---
drivers/nvdimm/pmem.c | 2 +-
drivers/scsi/osd/osd_initiator.c | 4 ++--
include/linux/blk-cgroup.h | 2 +-
include/linux/blkdev.h | 2 +-
include/linux/fs.h | 4 ++--
kernel/trace/blktrace.c | 2 +-
mm/filemap.c | 2 +-
21 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index a687e9c..ec5ed53 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2076,7 +2076,7 @@ blk_qc_t submit_bio(struct bio *bio)
else
count = bio_sectors(bio);
- if (op_is_write(bio_op(bio))) {
+ if (req_op_is_write(bio_op(bio))) {
count_vm_events(PGPGOUT, count);
} else {
task_io_account_read(bio->bi_iter.bi_size);
@@ -2087,7 +2087,7 @@ blk_qc_t submit_bio(struct bio *bio)
char b[BDEVNAME_SIZE];
printk(KERN_DEBUG "%s(%d): %s block %Lu on %s (%u sectors)\n",
current->comm, task_pid_nr(current),
- op_is_write(bio_op(bio)) ? "WRITE" : "READ",
+ req_op_is_write(bio_op(bio)) ? "WRITE" : "READ",
(unsigned long long)bio->bi_iter.bi_sector,
bdevname(bio->bi_bdev, b),
count);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 41cbd48..dcb646f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -439,7 +439,7 @@ int blk_rq_map_sg(struct request_queue *q, struct request *rq,
}
if (q->dma_drain_size && q->dma_drain_needed(rq)) {
- if (op_is_write(req_op(rq)))
+ if (req_op_is_write(req_op(rq)))
memset(q->dma_drain_buffer, 0, q->dma_drain_size);
sg_unmark_end(sg);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e207b33..3401881 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1190,7 +1190,7 @@ static int atapi_drain_needed(struct request *rq)
if (likely(rq->cmd_type != REQ_TYPE_BLOCK_PC))
return 0;
- if (!blk_rq_bytes(rq) || op_is_write(req_op(rq)))
+ if (!blk_rq_bytes(rq) || req_op_is_write(req_op(rq)))
return 0;
return atapi_cmd_type(rq->cmd[0]) == ATAPI_MISC;
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3439b28..c2dd622 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -306,14 +306,14 @@ static int brd_do_bvec(struct brd_device *brd, struct page *page,
void *mem;
int err = 0;
- if (op_is_write(op)) {
+ if (req_op_is_write(op)) {
err = copy_to_brd_setup(brd, sector, len);
if (err)
goto out;
}
mem = kmap_atomic(page);
- if (!op_is_write(op)) {
+ if (!req_op_is_write(op)) {
copy_from_brd(mem + off, brd, sector, len);
flush_dcache_page(page);
} else {
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 66b8e4b..8b0483a 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -265,7 +265,7 @@ void drbd_req_complete(struct drbd_request *req, struct bio_and_error *m)
* epoch number. If they match, increase the current_tle_nr,
* and reset the transfer log epoch write_cnt.
*/
- if (op_is_write(bio_op(req->master_bio)) &&
+ if (req_op_is_write(bio_op(req->master_bio)) &&
req->epoch == atomic_read(&first_peer_device(device)->connection->current_tle_nr))
start_new_tl_epoch(first_peer_device(device)->connection);
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index c9f2107..5edfd91 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -447,7 +447,7 @@ static int lo_req_flush(struct loop_device *lo, struct request *rq)
static inline void handle_partial_read(struct loop_cmd *cmd, long bytes)
{
- if (bytes < 0 || op_is_write(req_op(cmd->rq)))
+ if (bytes < 0 || req_op_is_write(req_op(cmd->rq)))
return;
if (unlikely(bytes < blk_rq_bytes(cmd->rq))) {
@@ -1665,7 +1665,7 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
static void loop_handle_cmd(struct loop_cmd *cmd)
{
- const bool write = op_is_write(req_op(cmd->rq));
+ const bool write = req_op_is_write(req_op(cmd->rq));
struct loop_device *lo = cmd->rq->q->queuedata;
int ret = 0;
diff --git a/drivers/block/umem.c b/drivers/block/umem.c
index d0a3e6d..3125368 100644
--- a/drivers/block/umem.c
+++ b/drivers/block/umem.c
@@ -460,7 +460,7 @@ static void process_page(unsigned long data)
le32_to_cpu(desc->local_addr)>>9,
le32_to_cpu(desc->transfer_size));
dump_dmastat(card, control);
- } else if (op_is_write(bio_op(bio)) &&
+ } else if (req_op_is_write(bio_op(bio)) &&
le32_to_cpu(desc->local_addr) >> 9 ==
card->init_size) {
card->init_size += le32_to_cpu(desc->transfer_size) >> 9;
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ca29649..b3d5e6e 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -851,7 +851,7 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
generic_start_io_acct(op, bvec->bv_len >> SECTOR_SHIFT,
&zram->disk->part0);
- if (!op_is_write(op)) {
+ if (!req_op_is_write(op)) {
atomic64_inc(&zram->stats.num_reads);
ret = zram_bvec_read(zram, bvec, index, offset);
} else {
@@ -862,7 +862,7 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
generic_end_io_acct(op, &zram->disk->part0, start_time);
if (unlikely(ret)) {
- if (!op_is_write(op))
+ if (!req_op_is_write(op))
atomic64_inc(&zram->stats.failed_reads);
else
atomic64_inc(&zram->stats.failed_writes);
diff --git a/drivers/md/bcache/io.c b/drivers/md/bcache/io.c
index e97b0ac..0b3ffe2 100644
--- a/drivers/md/bcache/io.c
+++ b/drivers/md/bcache/io.c
@@ -110,7 +110,7 @@ void bch_bbio_count_io_errors(struct cache_set *c, struct bio *bio,
struct bbio *b = container_of(bio, struct bbio, bio);
struct cache *ca = PTR_CACHE(c, &b->key, 0);
- unsigned threshold = op_is_write(bio_op(bio))
+ unsigned threshold = req_op_is_write(bio_op(bio))
? c->congested_write_threshold_us
: c->congested_read_threshold_us;
diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 69f16f4..ee3f8e6 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -383,7 +383,7 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio)
if (mode == CACHE_MODE_NONE ||
(mode == CACHE_MODE_WRITEAROUND &&
- op_is_write(bio_op(bio))))
+ req_op_is_write(bio_op(bio))))
goto skip;
if (bio->bi_iter.bi_sector & (c->sb.block_size - 1) ||
@@ -404,7 +404,7 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio)
if (!congested &&
mode == CACHE_MODE_WRITEBACK &&
- op_is_write(bio_op(bio)) &&
+ req_op_is_write(bio_op(bio)) &&
(bio->bi_rw & REQ_SYNC))
goto rescale;
@@ -657,7 +657,7 @@ static inline struct search *search_alloc(struct bio *bio,
s->cache_miss = NULL;
s->d = d;
s->recoverable = 1;
- s->write = op_is_write(bio_op(bio));
+ s->write = req_op_is_write(bio_op(bio));
s->read_dirty_data = 0;
s->start_time = jiffies;
diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c
index daa03e4..4ce39dc 100644
--- a/drivers/md/dm-io.c
+++ b/drivers/md/dm-io.c
@@ -411,7 +411,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,
struct io *io;
struct sync_io sio;
- if (num_regions > 1 && !op_is_write(op)) {
+ if (num_regions > 1 && !req_op_is_write(op)) {
WARN_ON(1);
return -EIO;
}
@@ -444,7 +444,7 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,
{
struct io *io;
- if (num_regions > 1 && !op_is_write(op)) {
+ if (num_regions > 1 && !req_op_is_write(op)) {
WARN_ON(1);
fn(1, context);
return -EIO;
diff --git a/drivers/md/dm-kcopyd.c b/drivers/md/dm-kcopyd.c
index 9e9d04cb..cd84769 100644
--- a/drivers/md/dm-kcopyd.c
+++ b/drivers/md/dm-kcopyd.c
@@ -465,7 +465,7 @@ static void complete_io(unsigned long error, void *context)
io_job_finish(kc->throttle);
if (error) {
- if (op_is_write(job->rw))
+ if (req_op_is_write(job->rw))
job->write_err |= error;
else
job->read_err = 1;
@@ -477,7 +477,7 @@ static void complete_io(unsigned long error, void *context)
}
}
- if (op_is_write(job->rw))
+ if (req_op_is_write(job->rw))
push(&kc->complete_jobs, job);
else {
@@ -551,7 +551,7 @@ static int process_jobs(struct list_head *jobs, struct dm_kcopyd_client *kc,
if (r < 0) {
/* error this rogue job */
- if (op_is_write(job->rw))
+ if (req_op_is_write(job->rw))
job->write_err = (unsigned long) -1L;
else
job->read_err = 1;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index d189e89..5c38378 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -927,7 +927,7 @@ again:
rdev = rrdev;
rrdev = NULL;
}
- if (op_is_write(op)) {
+ if (req_op_is_write(op)) {
if (replace_only)
rdev = NULL;
if (rdev == rrdev)
@@ -953,7 +953,7 @@ again:
* need to check for writes. We never accept write errors
* on the replacement, so we don't to check rrdev.
*/
- while (op_is_write(op) && rdev &&
+ while (req_op_is_write(op) && rdev &&
test_bit(WriteErrorSeen, &rdev->flags)) {
sector_t first_bad;
int bad_sectors;
@@ -996,7 +996,7 @@ again:
bio_reset(bi);
bi->bi_bdev = rdev->bdev;
bio_set_op_attrs(bi, op, op_flags);
- bi->bi_end_io = op_is_write(op)
+ bi->bi_end_io = req_op_is_write(op)
? raid5_end_write_request
: raid5_end_read_request;
bi->bi_private = sh;
@@ -1048,7 +1048,7 @@ again:
bio_reset(rbi);
rbi->bi_bdev = rrdev->bdev;
bio_set_op_attrs(rbi, op, op_flags);
- BUG_ON(!op_is_write(op));
+ BUG_ON(!req_op_is_write(op));
rbi->bi_end_io = raid5_end_write_request;
rbi->bi_private = sh;
@@ -1085,7 +1085,7 @@ again:
generic_make_request(rbi);
}
if (!rdev && !rrdev) {
- if (op_is_write(op))
+ if (req_op_is_write(op))
set_bit(STRIPE_DEGRADED, &sh->state);
pr_debug("skip op %d on disc %d for sector %llu\n",
bi->bi_rw, i, (unsigned long long)sh->sector);
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 7cf3bdf..68954550 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1137,7 +1137,7 @@ static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
{
int ret;
- if (!op_is_write(op)) {
+ if (!req_op_is_write(op)) {
ret = btt_read_pg(btt, bip, page, off, sector, len);
flush_dcache_page(page);
} else {
@@ -1184,8 +1184,8 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
if (err) {
dev_info(&btt->nd_btt->dev,
"io error in %s sector %lld, len %d,\n",
- (op_is_write(bio_op(bio))) ? "WRITE" :
- "READ",
+ (req_op_is_write(bio_op(bio))) ?
+ "WRITE" : "READ",
(unsigned long long) iter.bi_sector, len);
bio->bi_error = err;
break;
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index d64d924..7d4e62b 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -79,7 +79,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
bad_pmem = true;
- if (!op_is_write(op)) {
+ if (!req_op_is_write(op)) {
if (unlikely(bad_pmem))
rc = -EIO;
else {
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 2f2a991..0685df0 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -824,7 +824,7 @@ void osd_req_write(struct osd_request *or,
{
_osd_req_encode_common(or, OSD_ACT_WRITE, obj, offset, len);
WARN_ON(or->out.bio || or->out.total_bytes);
- WARN_ON(!op_is_write(bio_op(bio)));
+ WARN_ON(!req_op_is_write(bio_op(bio)));
or->out.bio = bio;
or->out.total_bytes = len;
}
@@ -875,7 +875,7 @@ void osd_req_read(struct osd_request *or,
{
_osd_req_encode_common(or, OSD_ACT_READ, obj, offset, len);
WARN_ON(or->in.bio || or->in.total_bytes);
- WARN_ON(op_is_write(bio_op(bio)));
+ WARN_ON(req_op_is_write(bio_op(bio)));
or->in.bio = bio;
or->in.total_bytes = len;
}
diff --git a/include/linux/blk-cgroup.h b/include/linux/blk-cgroup.h
index f77150a..6fc5316 100644
--- a/include/linux/blk-cgroup.h
+++ b/include/linux/blk-cgroup.h
@@ -602,7 +602,7 @@ static inline void blkg_rwstat_add(struct blkg_rwstat *rwstat,
{
struct percpu_counter *cnt;
- if (op_is_write(op))
+ if (req_op_is_write(op))
cnt = &rwstat->cpu_cnt[BLKG_RWSTAT_WRITE];
else
cnt = &rwstat->cpu_cnt[BLKG_RWSTAT_READ];
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ccd68c0..bfc3cea 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -611,7 +611,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
#define list_entry_rq(ptr) list_entry((ptr), struct request, queuelist)
-#define rq_data_dir(rq) (op_is_write(req_op(rq)) ? WRITE : READ)
+#define rq_data_dir(rq) (req_op_is_write(req_op(rq)) ? WRITE : READ)
/*
* Driver can handle struct request, if it either has an old style
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 498255e..aebba9e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2480,7 +2480,7 @@ 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 *);
-static inline bool op_is_write(unsigned int op)
+static inline bool req_op_is_write(unsigned int op)
{
return op == REQ_OP_READ ? false : true;
}
@@ -2492,7 +2492,7 @@ static inline bool op_is_write(unsigned int op)
*/
static inline int bio_data_dir(struct bio *bio)
{
- return op_is_write(bio_op(bio)) ? WRITE : READ;
+ return req_op_is_write(bio_op(bio)) ? WRITE : READ;
}
extern void check_disk_size_change(struct gendisk *disk,
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index fb345cd..434ee6d 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -217,7 +217,7 @@ static void __blk_add_trace(struct blk_trace *bt, sector_t sector, int bytes,
if (unlikely(bt->trace_state != Blktrace_running && !blk_tracer))
return;
- what |= ddir_act[op_is_write(op) ? WRITE : READ];
+ what |= ddir_act[req_op_is_write(op) ? WRITE : READ];
what |= MASK_TC_BIT(op_flags, SYNC);
what |= MASK_TC_BIT(op_flags, RAHEAD);
what |= MASK_TC_BIT(op_flags, META);
diff --git a/mm/filemap.c b/mm/filemap.c
index daef091..d891795 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -889,7 +889,7 @@ EXPORT_SYMBOL(end_page_writeback);
*/
void page_endio(struct page *page, int op, int err)
{
- if (!op_is_write(op)) {
+ if (!req_op_is_write(op)) {
if (!err) {
SetPageUptodate(page);
} else {
--
2.7.2
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.
>
> The rw_page users were not converted to use bio/req ops. As a result
> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
> be sent down as reads.
Can we just get rid of REQ_OP_WRITE enum for the ->rw_page interface
and pass a 'bool write'? If not I'd prefer to avoid using op_is_write
as much as possible - it's a confusing interface if we really want
to do a switch on read vs write vs invalid.
On 08/05/2016 01:57 AM, Christoph Hellwig wrote:
>>
>> The rw_page users were not converted to use bio/req ops. As a result
>> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
>> be sent down as reads.
>
> Can we just get rid of REQ_OP_WRITE enum for the ->rw_page interface
> and pass a 'bool write'? If not I'd prefer to avoid using op_is_write
> as much as possible - it's a confusing interface if we really want
> to do a switch on read vs write vs invalid.
That's a really good point, especially since the op_flags was dropped as
well. I'll cook that up.
--
Jens Axboe
On 08/05/2016 07:52 AM, Jens Axboe wrote:
> On 08/05/2016 01:57 AM, Christoph Hellwig wrote:
>>>
>>> The rw_page users were not converted to use bio/req ops. As a result
>>> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
>>> be sent down as reads.
>>
>> Can we just get rid of REQ_OP_WRITE enum for the ->rw_page interface
>> and pass a 'bool write'? If not I'd prefer to avoid using op_is_write
>> as much as possible - it's a confusing interface if we really want
>> to do a switch on read vs write vs invalid.
>
> That's a really good point, especially since the op_flags was dropped as
> well. I'll cook that up.
How about something like this? We don't bleed into the non-core bits
anymore, so we can move those back under CONFIG_BLOCK too.
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3439b28cce8b..0c76d4016eeb 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -300,20 +300,20 @@ static void copy_from_brd(void *dst, struct
brd_device *brd,
* Process a single bvec of a bio.
*/
static int brd_do_bvec(struct brd_device *brd, struct page *page,
- unsigned int len, unsigned int off, int op,
+ unsigned int len, unsigned int off, bool is_write,
sector_t sector)
{
void *mem;
int err = 0;
- if (op_is_write(op)) {
+ if (is_write) {
err = copy_to_brd_setup(brd, sector, len);
if (err)
goto out;
}
mem = kmap_atomic(page);
- if (!op_is_write(op)) {
+ if (!is_write) {
copy_from_brd(mem + off, brd, sector, len);
flush_dcache_page(page);
} else {
@@ -350,8 +350,8 @@ static blk_qc_t brd_make_request(struct
request_queue *q, struct bio *bio)
unsigned int len = bvec.bv_len;
int err;
- err = brd_do_bvec(brd, bvec.bv_page, len,
- bvec.bv_offset, bio_op(bio), sector);
+ err = brd_do_bvec(brd, bvec.bv_page, len, bvec.bv_offset,
+ op_is_write(bio_op(bio)), sector);
if (err)
goto io_error;
sector += len >> SECTOR_SHIFT;
@@ -366,11 +366,11 @@ io_error:
}
static int brd_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int op)
+ struct page *page, bool is_write)
{
struct brd_device *brd = bdev->bd_disk->private_data;
- int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, op, sector);
- page_endio(page, op, err);
+ int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, is_write, sector);
+ page_endio(page, is_write, err);
return err;
}
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index ca29649c4b08..04365b17ee67 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -843,15 +843,16 @@ static void zram_bio_discard(struct zram *zram,
u32 index,
}
static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32
index,
- int offset, int op)
+ int offset, bool is_write)
{
unsigned long start_time = jiffies;
+ int rw_acct = is_write ? REQ_OP_WRITE : REQ_OP_READ;
int ret;
- generic_start_io_acct(op, bvec->bv_len >> SECTOR_SHIFT,
+ generic_start_io_acct(rw_acct, bvec->bv_len >> SECTOR_SHIFT,
&zram->disk->part0);
- if (!op_is_write(op)) {
+ if (!is_write) {
atomic64_inc(&zram->stats.num_reads);
ret = zram_bvec_read(zram, bvec, index, offset);
} else {
@@ -859,10 +860,10 @@ static int zram_bvec_rw(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = zram_bvec_write(zram, bvec, index, offset);
}
- generic_end_io_acct(op, &zram->disk->part0, start_time);
+ generic_end_io_acct(rw_acct, &zram->disk->part0, start_time);
if (unlikely(ret)) {
- if (!op_is_write(op))
+ if (!is_write)
atomic64_inc(&zram->stats.failed_reads);
else
atomic64_inc(&zram->stats.failed_writes);
@@ -903,17 +904,17 @@ static void __zram_make_request(struct zram *zram,
struct bio *bio)
bv.bv_offset = bvec.bv_offset;
if (zram_bvec_rw(zram, &bv, index, offset,
- bio_op(bio)) < 0)
+ op_is_write(bio_op(bio))) < 0)
goto out;
bv.bv_len = bvec.bv_len - max_transfer_size;
bv.bv_offset += max_transfer_size;
if (zram_bvec_rw(zram, &bv, index + 1, 0,
- bio_op(bio)) < 0)
+ op_is_write(bio_op(bio))) < 0)
goto out;
} else
if (zram_bvec_rw(zram, &bvec, index, offset,
- bio_op(bio)) < 0)
+ op_is_write(bio_op(bio))) < 0)
goto out;
update_position(&index, &offset, &bvec);
@@ -970,7 +971,7 @@ static void zram_slot_free_notify(struct
block_device *bdev,
}
static int zram_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int op)
+ struct page *page, bool is_write)
{
int offset, err = -EIO;
u32 index;
@@ -994,7 +995,7 @@ static int zram_rw_page(struct block_device *bdev,
sector_t sector,
bv.bv_len = PAGE_SIZE;
bv.bv_offset = 0;
- err = zram_bvec_rw(zram, &bv, index, offset, op);
+ err = zram_bvec_rw(zram, &bv, index, offset, is_write);
put_zram:
zram_meta_put(zram);
out:
@@ -1007,7 +1008,7 @@ out:
* (e.g., SetPageError, set_page_dirty and extra works).
*/
if (err == 0)
- page_endio(page, op, 0);
+ page_endio(page, is_write, 0);
return err;
}
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 7cf3bdfaf809..88e91666f145 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1133,11 +1133,11 @@ static int btt_write_pg(struct btt *btt, struct
bio_integrity_payload *bip,
static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
struct page *page, unsigned int len, unsigned int off,
- int op, sector_t sector)
+ bool is_write, sector_t sector)
{
int ret;
- if (!op_is_write(op)) {
+ if (!is_write) {
ret = btt_read_pg(btt, bip, page, off, sector, len);
flush_dcache_page(page);
} else {
@@ -1180,7 +1180,7 @@ static blk_qc_t btt_make_request(struct
request_queue *q, struct bio *bio)
BUG_ON(len % btt->sector_size);
err = btt_do_bvec(btt, bip, bvec.bv_page, len, bvec.bv_offset,
- bio_op(bio), iter.bi_sector);
+ op_is_write(bio_op(bio)), iter.bi_sector);
if (err) {
dev_info(&btt->nd_btt->dev,
"io error in %s sector %lld, len %d,\n",
@@ -1200,12 +1200,12 @@ out:
}
static int btt_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int op)
+ struct page *page, bool is_write)
{
struct btt *btt = bdev->bd_disk->private_data;
- btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, op, sector);
- page_endio(page, op, 0);
+ btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, is_write, sector);
+ page_endio(page, is_write, 0);
return 0;
}
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index d64d92481c1d..20bae50c231d 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -67,7 +67,7 @@ static void pmem_clear_poison(struct pmem_device
*pmem, phys_addr_t offset,
}
static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
- unsigned int len, unsigned int off, int op,
+ unsigned int len, unsigned int off, bool is_write,
sector_t sector)
{
int rc = 0;
@@ -79,7 +79,7 @@ static int pmem_do_bvec(struct pmem_device *pmem,
struct page *page,
if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
bad_pmem = true;
- if (!op_is_write(op)) {
+ if (!is_write) {
if (unlikely(bad_pmem))
rc = -EIO;
else {
@@ -134,7 +134,7 @@ static blk_qc_t pmem_make_request(struct
request_queue *q, struct bio *bio)
do_acct = nd_iostat_start(bio, &start);
bio_for_each_segment(bvec, bio, iter) {
rc = pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len,
- bvec.bv_offset, bio_op(bio),
+ bvec.bv_offset, op_is_write(bio_op(bio)),
iter.bi_sector);
if (rc) {
bio->bi_error = rc;
@@ -152,12 +152,12 @@ static blk_qc_t pmem_make_request(struct
request_queue *q, struct bio *bio)
}
static int pmem_rw_page(struct block_device *bdev, sector_t sector,
- struct page *page, int op)
+ struct page *page, bool is_write)
{
struct pmem_device *pmem = bdev->bd_queue->queuedata;
int rc;
- rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, op, sector);
+ rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, is_write, sector);
/*
* The ->rw_page interface is subtle and tricky. The core
@@ -166,7 +166,7 @@ static int pmem_rw_page(struct block_device *bdev,
sector_t sector,
* caused by double completion.
*/
if (rc == 0)
- page_endio(page, op, 0);
+ page_endio(page, is_write, 0);
return rc;
}
diff --git a/fs/block_dev.c b/fs/block_dev.c
index d402899ba135..c3cdde87cc8c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -416,8 +416,7 @@ int bdev_read_page(struct block_device *bdev,
sector_t sector,
result = blk_queue_enter(bdev->bd_queue, false);
if (result)
return result;
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
- REQ_OP_READ);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, false);
blk_queue_exit(bdev->bd_queue);
return result;
}
@@ -455,8 +454,7 @@ int bdev_write_page(struct block_device *bdev,
sector_t sector,
return result;
set_page_writeback(page);
- result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
- REQ_OP_WRITE);
+ result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, true);
if (result)
end_page_writeback(page);
else
diff --git a/fs/mpage.c b/fs/mpage.c
index 7a09c55b4bd0..d2413af0823a 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -50,7 +50,7 @@ static void mpage_end_io(struct bio *bio)
bio_for_each_segment_all(bv, bio, i) {
struct page *page = bv->bv_page;
- page_endio(page, bio_op(bio), bio->bi_error);
+ page_endio(page, op_is_write(bio_op(bio)), bio->bi_error);
}
bio_put(bio);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 14b28ff2caf8..f254eb264924 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -18,17 +18,6 @@ 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
@@ -239,6 +228,17 @@ 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/blkdev.h b/include/linux/blkdev.h
index ccd68c0d01de..2c210b6a7bcf 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1672,7 +1672,7 @@ struct blk_dax_ctl {
struct block_device_operations {
int (*open) (struct block_device *, fmode_t);
void (*release) (struct gendisk *, fmode_t);
- int (*rw_page)(struct block_device *, sector_t, struct page *, int op);
+ int (*rw_page)(struct block_device *, sector_t, struct page *, bool);
int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
int (*compat_ioctl) (struct block_device *, fmode_t, unsigned,
unsigned long);
long (*direct_access)(struct block_device *, sector_t, void **, pfn_t *,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 498255e6914e..f3f0b4c8e8ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2480,13 +2480,12 @@ 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
*/
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 45786374abbd..66a1260b33de 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -510,7 +510,7 @@ static inline void wait_on_page_writeback(struct
page *page)
extern void end_page_writeback(struct page *page);
void wait_for_stable_page(struct page *page);
-void page_endio(struct page *page, int op, int err);
+void page_endio(struct page *page, bool is_write, int err);
/*
* Add an arbitrary waiter to a page's wait queue
diff --git a/mm/filemap.c b/mm/filemap.c
index daef091d4c50..8a287dfc5372 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -887,9 +887,9 @@ EXPORT_SYMBOL(end_page_writeback);
* After completing I/O on a page, call this routine to update the page
* flags appropriately
*/
-void page_endio(struct page *page, int op, int err)
+void page_endio(struct page *page, bool is_write, int err)
{
- if (!op_is_write(op)) {
+ if (!is_write) {
if (!err) {
SetPageUptodate(page);
} else {
--
Jens Axboe
On 08/05/2016 09:09 AM, Jens Axboe wrote:
> On 08/05/2016 07:52 AM, Jens Axboe wrote:
>> On 08/05/2016 01:57 AM, Christoph Hellwig wrote:
>>>>
>>>> The rw_page users were not converted to use bio/req ops. As a result
>>>> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
>>>> be sent down as reads.
>>>
>>> Can we just get rid of REQ_OP_WRITE enum for the ->rw_page interface
>>> and pass a 'bool write'? If not I'd prefer to avoid using op_is_write
>>> as much as possible - it's a confusing interface if we really want
>>> to do a switch on read vs write vs invalid.
>>
>> That's a really good point, especially since the op_flags was dropped as
>> well. I'll cook that up.
>
> How about something like this? We don't bleed into the non-core bits
> anymore, so we can move those back under CONFIG_BLOCK too.
>
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index 3439b28cce8b..0c76d4016eeb 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -300,20 +300,20 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
> * Process a single bvec of a bio.
> */
> static int brd_do_bvec(struct brd_device *brd, struct page *page,
> - unsigned int len, unsigned int off, int op,
> + unsigned int len, unsigned int off, bool is_write,
> sector_t sector)
> {
> void *mem;
> int err = 0;
>
> - if (op_is_write(op)) {
> + if (is_write) {
> err = copy_to_brd_setup(brd, sector, len);
> if (err)
> goto out;
> }
>
> mem = kmap_atomic(page);
> - if (!op_is_write(op)) {
> + if (!is_write) {
> copy_from_brd(mem + off, brd, sector, len);
> flush_dcache_page(page);
> } else {
> @@ -350,8 +350,8 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
> unsigned int len = bvec.bv_len;
> int err;
>
> - err = brd_do_bvec(brd, bvec.bv_page, len,
> - bvec.bv_offset, bio_op(bio), sector);
> + err = brd_do_bvec(brd, bvec.bv_page, len, bvec.bv_offset,
> + op_is_write(bio_op(bio)), sector);
> if (err)
> goto io_error;
> sector += len >> SECTOR_SHIFT;
> @@ -366,11 +366,11 @@ io_error:
> }
>
> static int brd_rw_page(struct block_device *bdev, sector_t sector,
> - struct page *page, int op)
> + struct page *page, bool is_write)
> {
> struct brd_device *brd = bdev->bd_disk->private_data;
> - int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, op, sector);
> - page_endio(page, op, err);
> + int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, is_write, sector);
> + page_endio(page, is_write, err);
> return err;
> }
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ca29649c4b08..04365b17ee67 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -843,15 +843,16 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> }
>
> static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
> - int offset, int op)
> + int offset, bool is_write)
> {
> unsigned long start_time = jiffies;
> + int rw_acct = is_write ? REQ_OP_WRITE : REQ_OP_READ;
> int ret;
>
> - generic_start_io_acct(op, bvec->bv_len >> SECTOR_SHIFT,
> + generic_start_io_acct(rw_acct, bvec->bv_len >> SECTOR_SHIFT,
> &zram->disk->part0);
>
> - if (!op_is_write(op)) {
> + if (!is_write) {
> atomic64_inc(&zram->stats.num_reads);
> ret = zram_bvec_read(zram, bvec, index, offset);
> } else {
> @@ -859,10 +860,10 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
> ret = zram_bvec_write(zram, bvec, index, offset);
> }
>
> - generic_end_io_acct(op, &zram->disk->part0, start_time);
> + generic_end_io_acct(rw_acct, &zram->disk->part0, start_time);
>
> if (unlikely(ret)) {
> - if (!op_is_write(op))
> + if (!is_write)
> atomic64_inc(&zram->stats.failed_reads);
> else
> atomic64_inc(&zram->stats.failed_writes);
> @@ -903,17 +904,17 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
> bv.bv_offset = bvec.bv_offset;
>
> if (zram_bvec_rw(zram, &bv, index, offset,
> - bio_op(bio)) < 0)
> + op_is_write(bio_op(bio))) < 0)
> goto out;
>
> bv.bv_len = bvec.bv_len - max_transfer_size;
> bv.bv_offset += max_transfer_size;
> if (zram_bvec_rw(zram, &bv, index + 1, 0,
> - bio_op(bio)) < 0)
> + op_is_write(bio_op(bio))) < 0)
> goto out;
> } else
> if (zram_bvec_rw(zram, &bvec, index, offset,
> - bio_op(bio)) < 0)
> + op_is_write(bio_op(bio))) < 0)
> goto out;
>
> update_position(&index, &offset, &bvec);
> @@ -970,7 +971,7 @@ static void zram_slot_free_notify(struct block_device *bdev,
> }
>
> static int zram_rw_page(struct block_device *bdev, sector_t sector,
> - struct page *page, int op)
> + struct page *page, bool is_write)
> {
> int offset, err = -EIO;
> u32 index;
> @@ -994,7 +995,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
> bv.bv_len = PAGE_SIZE;
> bv.bv_offset = 0;
>
> - err = zram_bvec_rw(zram, &bv, index, offset, op);
> + err = zram_bvec_rw(zram, &bv, index, offset, is_write);
> put_zram:
> zram_meta_put(zram);
> out:
> @@ -1007,7 +1008,7 @@ out:
> * (e.g., SetPageError, set_page_dirty and extra works).
> */
> if (err == 0)
> - page_endio(page, op, 0);
> + page_endio(page, is_write, 0);
> return err;
> }
>
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index 7cf3bdfaf809..88e91666f145 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -1133,11 +1133,11 @@ static int btt_write_pg(struct btt *btt, struct bio_integrity_payload *bip,
>
> static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
> struct page *page, unsigned int len, unsigned int off,
> - int op, sector_t sector)
> + bool is_write, sector_t sector)
> {
> int ret;
>
> - if (!op_is_write(op)) {
> + if (!is_write) {
> ret = btt_read_pg(btt, bip, page, off, sector, len);
> flush_dcache_page(page);
> } else {
> @@ -1180,7 +1180,7 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
> BUG_ON(len % btt->sector_size);
>
> err = btt_do_bvec(btt, bip, bvec.bv_page, len, bvec.bv_offset,
> - bio_op(bio), iter.bi_sector);
> + op_is_write(bio_op(bio)), iter.bi_sector);
> if (err) {
> dev_info(&btt->nd_btt->dev,
> "io error in %s sector %lld, len %d,\n",
> @@ -1200,12 +1200,12 @@ out:
> }
>
> static int btt_rw_page(struct block_device *bdev, sector_t sector,
> - struct page *page, int op)
> + struct page *page, bool is_write)
> {
> struct btt *btt = bdev->bd_disk->private_data;
>
> - btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, op, sector);
> - page_endio(page, op, 0);
> + btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, is_write, sector);
> + page_endio(page, is_write, 0);
> return 0;
> }
>
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index d64d92481c1d..20bae50c231d 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -67,7 +67,7 @@ static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
> }
>
> static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
> - unsigned int len, unsigned int off, int op,
> + unsigned int len, unsigned int off, bool is_write,
> sector_t sector)
> {
> int rc = 0;
> @@ -79,7 +79,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
> if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
> bad_pmem = true;
>
> - if (!op_is_write(op)) {
> + if (!is_write) {
> if (unlikely(bad_pmem))
> rc = -EIO;
> else {
> @@ -134,7 +134,7 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
> do_acct = nd_iostat_start(bio, &start);
> bio_for_each_segment(bvec, bio, iter) {
> rc = pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len,
> - bvec.bv_offset, bio_op(bio),
> + bvec.bv_offset, op_is_write(bio_op(bio)),
> iter.bi_sector);
> if (rc) {
> bio->bi_error = rc;
> @@ -152,12 +152,12 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
> }
>
> static int pmem_rw_page(struct block_device *bdev, sector_t sector,
> - struct page *page, int op)
> + struct page *page, bool is_write)
> {
> struct pmem_device *pmem = bdev->bd_queue->queuedata;
> int rc;
>
> - rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, op, sector);
> + rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, is_write, sector);
>
> /*
> * The ->rw_page interface is subtle and tricky. The core
> @@ -166,7 +166,7 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
> * caused by double completion.
> */
> if (rc == 0)
> - page_endio(page, op, 0);
> + page_endio(page, is_write, 0);
>
> return rc;
> }
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index d402899ba135..c3cdde87cc8c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -416,8 +416,7 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
> result = blk_queue_enter(bdev->bd_queue, false);
> if (result)
> return result;
> - result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
> - REQ_OP_READ);
> + result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, false);
> blk_queue_exit(bdev->bd_queue);
> return result;
> }
> @@ -455,8 +454,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
> return result;
>
> set_page_writeback(page);
> - result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
> - REQ_OP_WRITE);
> + result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, true);
> if (result)
> end_page_writeback(page);
> else
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 7a09c55b4bd0..d2413af0823a 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -50,7 +50,7 @@ static void mpage_end_io(struct bio *bio)
>
> bio_for_each_segment_all(bv, bio, i) {
> struct page *page = bv->bv_page;
> - page_endio(page, bio_op(bio), bio->bi_error);
> + page_endio(page, op_is_write(bio_op(bio)), bio->bi_error);
> }
>
> bio_put(bio);
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 14b28ff2caf8..f254eb264924 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -18,17 +18,6 @@ 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
> @@ -239,6 +228,17 @@ 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/blkdev.h b/include/linux/blkdev.h
> index ccd68c0d01de..2c210b6a7bcf 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1672,7 +1672,7 @@ struct blk_dax_ctl {
> struct block_device_operations {
> int (*open) (struct block_device *, fmode_t);
> void (*release) (struct gendisk *, fmode_t);
> - int (*rw_page)(struct block_device *, sector_t, struct page *, int op);
> + int (*rw_page)(struct block_device *, sector_t, struct page *, bool);
> int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
> int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
> long (*direct_access)(struct block_device *, sector_t, void **, pfn_t *,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 498255e6914e..f3f0b4c8e8ac 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2480,13 +2480,12 @@ 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
> */
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 45786374abbd..66a1260b33de 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -510,7 +510,7 @@ static inline void wait_on_page_writeback(struct page *page)
> extern void end_page_writeback(struct page *page);
> void wait_for_stable_page(struct page *page);
>
> -void page_endio(struct page *page, int op, int err);
> +void page_endio(struct page *page, bool is_write, int err);
>
> /*
> * Add an arbitrary waiter to a page's wait queue
> diff --git a/mm/filemap.c b/mm/filemap.c
> index daef091d4c50..8a287dfc5372 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -887,9 +887,9 @@ EXPORT_SYMBOL(end_page_writeback);
> * After completing I/O on a page, call this routine to update the page
> * flags appropriately
> */
> -void page_endio(struct page *page, int op, int err)
> +void page_endio(struct page *page, bool is_write, int err)
> {
> - if (!op_is_write(op)) {
> + if (!is_write) {
> if (!err) {
> SetPageUptodate(page);
> } else {
>
Looks ok to me. Also tested all the patches in the linus2 branch
and verified it fixed the rw_page related hang for me.
Reviewed-by: Mike Christie <[email protected]>
On 08/05/2016 12:05 PM, Mike Christie wrote:
> On 08/05/2016 09:09 AM, Jens Axboe wrote:
>> On 08/05/2016 07:52 AM, Jens Axboe wrote:
>>> On 08/05/2016 01:57 AM, Christoph Hellwig wrote:
>>>>>
>>>>> The rw_page users were not converted to use bio/req ops. As a result
>>>>> bdev_write_page is not passing down REQ_OP_WRITE and the IOs will
>>>>> be sent down as reads.
>>>>
>>>> Can we just get rid of REQ_OP_WRITE enum for the ->rw_page interface
>>>> and pass a 'bool write'? If not I'd prefer to avoid using op_is_write
>>>> as much as possible - it's a confusing interface if we really want
>>>> to do a switch on read vs write vs invalid.
>>>
>>> That's a really good point, especially since the op_flags was dropped as
>>> well. I'll cook that up.
>>
>> How about something like this? We don't bleed into the non-core bits
>> anymore, so we can move those back under CONFIG_BLOCK too.
>>
>> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
>> index 3439b28cce8b..0c76d4016eeb 100644
>> --- a/drivers/block/brd.c
>> +++ b/drivers/block/brd.c
>> @@ -300,20 +300,20 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
>> * Process a single bvec of a bio.
>> */
>> static int brd_do_bvec(struct brd_device *brd, struct page *page,
>> - unsigned int len, unsigned int off, int op,
>> + unsigned int len, unsigned int off, bool is_write,
>> sector_t sector)
>> {
>> void *mem;
>> int err = 0;
>>
>> - if (op_is_write(op)) {
>> + if (is_write) {
>> err = copy_to_brd_setup(brd, sector, len);
>> if (err)
>> goto out;
>> }
>>
>> mem = kmap_atomic(page);
>> - if (!op_is_write(op)) {
>> + if (!is_write) {
>> copy_from_brd(mem + off, brd, sector, len);
>> flush_dcache_page(page);
>> } else {
>> @@ -350,8 +350,8 @@ static blk_qc_t brd_make_request(struct request_queue *q, struct bio *bio)
>> unsigned int len = bvec.bv_len;
>> int err;
>>
>> - err = brd_do_bvec(brd, bvec.bv_page, len,
>> - bvec.bv_offset, bio_op(bio), sector);
>> + err = brd_do_bvec(brd, bvec.bv_page, len, bvec.bv_offset,
>> + op_is_write(bio_op(bio)), sector);
>> if (err)
>> goto io_error;
>> sector += len >> SECTOR_SHIFT;
>> @@ -366,11 +366,11 @@ io_error:
>> }
>>
>> static int brd_rw_page(struct block_device *bdev, sector_t sector,
>> - struct page *page, int op)
>> + struct page *page, bool is_write)
>> {
>> struct brd_device *brd = bdev->bd_disk->private_data;
>> - int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, op, sector);
>> - page_endio(page, op, err);
>> + int err = brd_do_bvec(brd, page, PAGE_SIZE, 0, is_write, sector);
>> + page_endio(page, is_write, err);
>> return err;
>> }
>>
>> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
>> index ca29649c4b08..04365b17ee67 100644
>> --- a/drivers/block/zram/zram_drv.c
>> +++ b/drivers/block/zram/zram_drv.c
>> @@ -843,15 +843,16 @@ static void zram_bio_discard(struct zram *zram, u32 index,
>> }
>>
>> static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
>> - int offset, int op)
>> + int offset, bool is_write)
>> {
>> unsigned long start_time = jiffies;
>> + int rw_acct = is_write ? REQ_OP_WRITE : REQ_OP_READ;
>> int ret;
>>
>> - generic_start_io_acct(op, bvec->bv_len >> SECTOR_SHIFT,
>> + generic_start_io_acct(rw_acct, bvec->bv_len >> SECTOR_SHIFT,
>> &zram->disk->part0);
>>
>> - if (!op_is_write(op)) {
>> + if (!is_write) {
>> atomic64_inc(&zram->stats.num_reads);
>> ret = zram_bvec_read(zram, bvec, index, offset);
>> } else {
>> @@ -859,10 +860,10 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
>> ret = zram_bvec_write(zram, bvec, index, offset);
>> }
>>
>> - generic_end_io_acct(op, &zram->disk->part0, start_time);
>> + generic_end_io_acct(rw_acct, &zram->disk->part0, start_time);
>>
>> if (unlikely(ret)) {
>> - if (!op_is_write(op))
>> + if (!is_write)
>> atomic64_inc(&zram->stats.failed_reads);
>> else
>> atomic64_inc(&zram->stats.failed_writes);
>> @@ -903,17 +904,17 @@ static void __zram_make_request(struct zram *zram, struct bio *bio)
>> bv.bv_offset = bvec.bv_offset;
>>
>> if (zram_bvec_rw(zram, &bv, index, offset,
>> - bio_op(bio)) < 0)
>> + op_is_write(bio_op(bio))) < 0)
>> goto out;
>>
>> bv.bv_len = bvec.bv_len - max_transfer_size;
>> bv.bv_offset += max_transfer_size;
>> if (zram_bvec_rw(zram, &bv, index + 1, 0,
>> - bio_op(bio)) < 0)
>> + op_is_write(bio_op(bio))) < 0)
>> goto out;
>> } else
>> if (zram_bvec_rw(zram, &bvec, index, offset,
>> - bio_op(bio)) < 0)
>> + op_is_write(bio_op(bio))) < 0)
>> goto out;
>>
>> update_position(&index, &offset, &bvec);
>> @@ -970,7 +971,7 @@ static void zram_slot_free_notify(struct block_device *bdev,
>> }
>>
>> static int zram_rw_page(struct block_device *bdev, sector_t sector,
>> - struct page *page, int op)
>> + struct page *page, bool is_write)
>> {
>> int offset, err = -EIO;
>> u32 index;
>> @@ -994,7 +995,7 @@ static int zram_rw_page(struct block_device *bdev, sector_t sector,
>> bv.bv_len = PAGE_SIZE;
>> bv.bv_offset = 0;
>>
>> - err = zram_bvec_rw(zram, &bv, index, offset, op);
>> + err = zram_bvec_rw(zram, &bv, index, offset, is_write);
>> put_zram:
>> zram_meta_put(zram);
>> out:
>> @@ -1007,7 +1008,7 @@ out:
>> * (e.g., SetPageError, set_page_dirty and extra works).
>> */
>> if (err == 0)
>> - page_endio(page, op, 0);
>> + page_endio(page, is_write, 0);
>> return err;
>> }
>>
>> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
>> index 7cf3bdfaf809..88e91666f145 100644
>> --- a/drivers/nvdimm/btt.c
>> +++ b/drivers/nvdimm/btt.c
>> @@ -1133,11 +1133,11 @@ static int btt_write_pg(struct btt *btt, struct bio_integrity_payload *bip,
>>
>> static int btt_do_bvec(struct btt *btt, struct bio_integrity_payload *bip,
>> struct page *page, unsigned int len, unsigned int off,
>> - int op, sector_t sector)
>> + bool is_write, sector_t sector)
>> {
>> int ret;
>>
>> - if (!op_is_write(op)) {
>> + if (!is_write) {
>> ret = btt_read_pg(btt, bip, page, off, sector, len);
>> flush_dcache_page(page);
>> } else {
>> @@ -1180,7 +1180,7 @@ static blk_qc_t btt_make_request(struct request_queue *q, struct bio *bio)
>> BUG_ON(len % btt->sector_size);
>>
>> err = btt_do_bvec(btt, bip, bvec.bv_page, len, bvec.bv_offset,
>> - bio_op(bio), iter.bi_sector);
>> + op_is_write(bio_op(bio)), iter.bi_sector);
>> if (err) {
>> dev_info(&btt->nd_btt->dev,
>> "io error in %s sector %lld, len %d,\n",
>> @@ -1200,12 +1200,12 @@ out:
>> }
>>
>> static int btt_rw_page(struct block_device *bdev, sector_t sector,
>> - struct page *page, int op)
>> + struct page *page, bool is_write)
>> {
>> struct btt *btt = bdev->bd_disk->private_data;
>>
>> - btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, op, sector);
>> - page_endio(page, op, 0);
>> + btt_do_bvec(btt, NULL, page, PAGE_SIZE, 0, is_write, sector);
>> + page_endio(page, is_write, 0);
>> return 0;
>> }
>>
>> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
>> index d64d92481c1d..20bae50c231d 100644
>> --- a/drivers/nvdimm/pmem.c
>> +++ b/drivers/nvdimm/pmem.c
>> @@ -67,7 +67,7 @@ static void pmem_clear_poison(struct pmem_device *pmem, phys_addr_t offset,
>> }
>>
>> static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
>> - unsigned int len, unsigned int off, int op,
>> + unsigned int len, unsigned int off, bool is_write,
>> sector_t sector)
>> {
>> int rc = 0;
>> @@ -79,7 +79,7 @@ static int pmem_do_bvec(struct pmem_device *pmem, struct page *page,
>> if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
>> bad_pmem = true;
>>
>> - if (!op_is_write(op)) {
>> + if (!is_write) {
>> if (unlikely(bad_pmem))
>> rc = -EIO;
>> else {
>> @@ -134,7 +134,7 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
>> do_acct = nd_iostat_start(bio, &start);
>> bio_for_each_segment(bvec, bio, iter) {
>> rc = pmem_do_bvec(pmem, bvec.bv_page, bvec.bv_len,
>> - bvec.bv_offset, bio_op(bio),
>> + bvec.bv_offset, op_is_write(bio_op(bio)),
>> iter.bi_sector);
>> if (rc) {
>> bio->bi_error = rc;
>> @@ -152,12 +152,12 @@ static blk_qc_t pmem_make_request(struct request_queue *q, struct bio *bio)
>> }
>>
>> static int pmem_rw_page(struct block_device *bdev, sector_t sector,
>> - struct page *page, int op)
>> + struct page *page, bool is_write)
>> {
>> struct pmem_device *pmem = bdev->bd_queue->queuedata;
>> int rc;
>>
>> - rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, op, sector);
>> + rc = pmem_do_bvec(pmem, page, PAGE_SIZE, 0, is_write, sector);
>>
>> /*
>> * The ->rw_page interface is subtle and tricky. The core
>> @@ -166,7 +166,7 @@ static int pmem_rw_page(struct block_device *bdev, sector_t sector,
>> * caused by double completion.
>> */
>> if (rc == 0)
>> - page_endio(page, op, 0);
>> + page_endio(page, is_write, 0);
>>
>> return rc;
>> }
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index d402899ba135..c3cdde87cc8c 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -416,8 +416,7 @@ int bdev_read_page(struct block_device *bdev, sector_t sector,
>> result = blk_queue_enter(bdev->bd_queue, false);
>> if (result)
>> return result;
>> - result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
>> - REQ_OP_READ);
>> + result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, false);
>> blk_queue_exit(bdev->bd_queue);
>> return result;
>> }
>> @@ -455,8 +454,7 @@ int bdev_write_page(struct block_device *bdev, sector_t sector,
>> return result;
>>
>> set_page_writeback(page);
>> - result = ops->rw_page(bdev, sector + get_start_sect(bdev), page,
>> - REQ_OP_WRITE);
>> + result = ops->rw_page(bdev, sector + get_start_sect(bdev), page, true);
>> if (result)
>> end_page_writeback(page);
>> else
>> diff --git a/fs/mpage.c b/fs/mpage.c
>> index 7a09c55b4bd0..d2413af0823a 100644
>> --- a/fs/mpage.c
>> +++ b/fs/mpage.c
>> @@ -50,7 +50,7 @@ static void mpage_end_io(struct bio *bio)
>>
>> bio_for_each_segment_all(bv, bio, i) {
>> struct page *page = bv->bv_page;
>> - page_endio(page, bio_op(bio), bio->bi_error);
>> + page_endio(page, op_is_write(bio_op(bio)), bio->bi_error);
>> }
>>
>> bio_put(bio);
>> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
>> index 14b28ff2caf8..f254eb264924 100644
>> --- a/include/linux/blk_types.h
>> +++ b/include/linux/blk_types.h
>> @@ -18,17 +18,6 @@ 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
>> @@ -239,6 +228,17 @@ 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/blkdev.h b/include/linux/blkdev.h
>> index ccd68c0d01de..2c210b6a7bcf 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -1672,7 +1672,7 @@ struct blk_dax_ctl {
>> struct block_device_operations {
>> int (*open) (struct block_device *, fmode_t);
>> void (*release) (struct gendisk *, fmode_t);
>> - int (*rw_page)(struct block_device *, sector_t, struct page *, int op);
>> + int (*rw_page)(struct block_device *, sector_t, struct page *, bool);
>> int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
>> int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long);
>> long (*direct_access)(struct block_device *, sector_t, void **, pfn_t *,
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 498255e6914e..f3f0b4c8e8ac 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2480,13 +2480,12 @@ 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
>> */
>> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
>> index 45786374abbd..66a1260b33de 100644
>> --- a/include/linux/pagemap.h
>> +++ b/include/linux/pagemap.h
>> @@ -510,7 +510,7 @@ static inline void wait_on_page_writeback(struct page *page)
>> extern void end_page_writeback(struct page *page);
>> void wait_for_stable_page(struct page *page);
>>
>> -void page_endio(struct page *page, int op, int err);
>> +void page_endio(struct page *page, bool is_write, int err);
>>
>> /*
>> * Add an arbitrary waiter to a page's wait queue
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index daef091d4c50..8a287dfc5372 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -887,9 +887,9 @@ EXPORT_SYMBOL(end_page_writeback);
>> * After completing I/O on a page, call this routine to update the page
>> * flags appropriately
>> */
>> -void page_endio(struct page *page, int op, int err)
>> +void page_endio(struct page *page, bool is_write, int err)
>> {
>> - if (!op_is_write(op)) {
>> + if (!is_write) {
>> if (!err) {
>> SetPageUptodate(page);
>> } else {
>>
>
> Looks ok to me. Also tested all the patches in the linus2 branch
> and verified it fixed the rw_page related hang for me.
>
> Reviewed-by: Mike Christie <[email protected]>
Great, thanks Mike. I'll add that, I'm going to rebase it with the
bio_opf branch anyway.
--
Jens Axboe