diff mbox

Testing latest linux-next 4.9 ib_srp and ib_srpt sees I/O capped at 1MB and no merging

Message ID 20161222154049.GA4638@infradead.org (mailing list archive)
State Superseded
Headers show

Commit Message

Christoph Hellwig Dec. 22, 2016, 3:40 p.m. UTC 
Hi Laurance,

please try the patch below:

---
From 69febe1cfb55844862f768447432249781001f9c Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 22 Dec 2016 16:38:29 +0100
Subject: block: add back plugging in __blkdev_direct_IO

This allows sending larger than 1 MB requess to devices that support
large I/O sizes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Laurence Oberman <loberman@redhat.com>
---
 fs/block_dev.c | 3 +++
 fs/iomap.c     | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Laurence Oberman Dec. 22, 2016, 5:59 p.m. UTC | #1 
----- Original Message -----
> From: "Christoph Hellwig" <hch@infradead.org>
> To: "Laurence Oberman" <loberman@redhat.com>
> Cc: "Christoph Hellwig" <hch@infradead.org>, "Bart Van Assche" <Bart.VanAssche@sandisk.com>,
> linux-rdma@vger.kernel.org, linux-scsi@vger.kernel.org
> Sent: Thursday, December 22, 2016 10:40:49 AM
> Subject: Re: Testing latest linux-next 4.9 ib_srp and ib_srpt sees I/O capped at 1MB and no merging
> 
> Hi Laurance,
> 
> please try the patch below:
> 
> ---
> From 69febe1cfb55844862f768447432249781001f9c Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Thu, 22 Dec 2016 16:38:29 +0100
> Subject: block: add back plugging in __blkdev_direct_IO
> 
> This allows sending larger than 1 MB requess to devices that support
> large I/O sizes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Laurence Oberman <loberman@redhat.com>
> ---
>  fs/block_dev.c | 3 +++
>  fs/iomap.c     | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7c45072..206a92a 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -328,6 +328,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter
> *iter, int nr_pages)
>  	struct file *file = iocb->ki_filp;
>  	struct inode *inode = bdev_file_inode(file);
>  	struct block_device *bdev = I_BDEV(inode);
> +	struct blk_plug plug;
>  	struct blkdev_dio *dio;
>  	struct bio *bio;
>  	bool is_read = (iov_iter_rw(iter) == READ);
> @@ -353,6 +354,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter
> *iter, int nr_pages)
>  	dio->multi_bio = false;
>  	dio->should_dirty = is_read && (iter->type == ITER_IOVEC);
>  
> +	blk_start_plug(&plug);
>  	for (;;) {
>  		bio->bi_bdev = bdev;
>  		bio->bi_iter.bi_sector = pos >> 9;
> @@ -394,6 +396,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter
> *iter, int nr_pages)
>  		submit_bio(bio);
>  		bio = bio_alloc(GFP_KERNEL, nr_pages);
>  	}
> +	blk_finish_plug(&plug);
>  
>  	if (!dio->is_sync)
>  		return -EIOCBQUEUED;
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 354a123..3adf1e1 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -844,7 +844,6 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
> struct iomap_ops *ops,
>  	size_t count = iov_iter_count(iter);
>  	loff_t pos = iocb->ki_pos, end = iocb->ki_pos + count - 1, ret = 0;
>  	unsigned int flags = IOMAP_DIRECT;
> -	struct blk_plug plug;
>  	struct iomap_dio *dio;
>  
>  	lockdep_assert_held(&inode->i_rwsem);
> --
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Hello Christoph

The patch works and I now see 4MB I/O

# DISK STATISTICS (/sec)
#                   <---------reads---------><---------writes---------><--------averages--------> Pct
#Time     Name       KBytes Merged  IOs Size  KBytes Merged  IOs Size  RWSize  QLen  Wait SvcTim Util
11:53:58 sdah        143360    105   35 4096       0      0    0    0    4096     1    28     28   99
11:53:59 sdah        139264    102   34 4096       0      0    0    0    4096     1    29     29   99
11:54:00 sdah        143360    105   35 4096       0      0    0    0    4096     1    28     28   99

I think you forgot to remove calls to blk_start_plug and blk_finish_plug in fs/iomap.c in your patch.
I took them out and built the test kernel that way.

Let me know if you will just remove those in the final or want a patch.


Thanks fo rthe super quick response

Regards
Laurence
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot Dec. 22, 2016, 7:54 p.m. UTC | #2 
Hi Christoph,

[auto build test ERROR on linus/master]
[also build test ERROR on next-20161222]
[cannot apply to v4.9]
[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/Christoph-Hellwig/block-add-back-plugging-in-__blkdev_direct_IO/20161223-002453
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/iomap.c: In function 'iomap_dio_rw':
>> fs/iomap.c:897:18: error: 'plug' undeclared (first use in this function)
     blk_start_plug(&plug);
                     ^~~~
   fs/iomap.c:897:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/plug +897 fs/iomap.c

ff6a9292 Christoph Hellwig 2016-11-30  891  		WARN_ON_ONCE(ret);
ff6a9292 Christoph Hellwig 2016-11-30  892  		ret = 0;
ff6a9292 Christoph Hellwig 2016-11-30  893  	}
ff6a9292 Christoph Hellwig 2016-11-30  894  
ff6a9292 Christoph Hellwig 2016-11-30  895  	inode_dio_begin(inode);
ff6a9292 Christoph Hellwig 2016-11-30  896  
ff6a9292 Christoph Hellwig 2016-11-30 @897  	blk_start_plug(&plug);
ff6a9292 Christoph Hellwig 2016-11-30  898  	do {
ff6a9292 Christoph Hellwig 2016-11-30  899  		ret = iomap_apply(inode, pos, count, flags, ops, dio,
ff6a9292 Christoph Hellwig 2016-11-30  900  				iomap_dio_actor);

:::::: The code at line 897 was first introduced by commit
:::::: ff6a9292e6f633d596826be5ba70d3ef90cc3300 iomap: implement direct I/O

:::::: TO: Christoph Hellwig <hch@lst.de>
:::::: CC: Dave Chinner <david@fromorbit.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Dec. 22, 2016, 7:55 p.m. UTC | #3 
Hi Christoph,

[auto build test ERROR on linus/master]
[also build test ERROR on next-20161222]
[cannot apply to v4.9]
[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/Christoph-Hellwig/block-add-back-plugging-in-__blkdev_direct_IO/20161223-002453
config: x86_64-lkp (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/iomap.c: In function 'iomap_dio_rw':
>> fs/iomap.c:897:18: error: 'plug' undeclared (first use in this function)
     blk_start_plug(&plug);
                     ^~~~
   fs/iomap.c:897:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/plug +897 fs/iomap.c

ff6a9292 Christoph Hellwig 2016-11-30  891  		WARN_ON_ONCE(ret);
ff6a9292 Christoph Hellwig 2016-11-30  892  		ret = 0;
ff6a9292 Christoph Hellwig 2016-11-30  893  	}
ff6a9292 Christoph Hellwig 2016-11-30  894  
ff6a9292 Christoph Hellwig 2016-11-30  895  	inode_dio_begin(inode);
ff6a9292 Christoph Hellwig 2016-11-30  896  
ff6a9292 Christoph Hellwig 2016-11-30 @897  	blk_start_plug(&plug);
ff6a9292 Christoph Hellwig 2016-11-30  898  	do {
ff6a9292 Christoph Hellwig 2016-11-30  899  		ret = iomap_apply(inode, pos, count, flags, ops, dio,
ff6a9292 Christoph Hellwig 2016-11-30  900  				iomap_dio_actor);

:::::: The code at line 897 was first introduced by commit
:::::: ff6a9292e6f633d596826be5ba70d3ef90cc3300 iomap: implement direct I/O

:::::: TO: Christoph Hellwig <hch@lst.de>
:::::: CC: Dave Chinner <david@fromorbit.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7c45072..206a92a 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -328,6 +328,7 @@  __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = bdev_file_inode(file);
 	struct block_device *bdev = I_BDEV(inode);
+	struct blk_plug plug;
 	struct blkdev_dio *dio;
 	struct bio *bio;
 	bool is_read = (iov_iter_rw(iter) == READ);
@@ -353,6 +354,7 @@  __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
 	dio->multi_bio = false;
 	dio->should_dirty = is_read && (iter->type == ITER_IOVEC);
 
+	blk_start_plug(&plug);
 	for (;;) {
 		bio->bi_bdev = bdev;
 		bio->bi_iter.bi_sector = pos >> 9;
@@ -394,6 +396,7 @@  __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
 		submit_bio(bio);
 		bio = bio_alloc(GFP_KERNEL, nr_pages);
 	}
+	blk_finish_plug(&plug);
 
 	if (!dio->is_sync)
 		return -EIOCBQUEUED;
diff --git a/fs/iomap.c b/fs/iomap.c
index 354a123..3adf1e1 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -844,7 +844,6 @@  iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter, struct iomap_ops *ops,
 	size_t count = iov_iter_count(iter);
 	loff_t pos = iocb->ki_pos, end = iocb->ki_pos + count - 1, ret = 0;
 	unsigned int flags = IOMAP_DIRECT;
-	struct blk_plug plug;
 	struct iomap_dio *dio;
 
 	lockdep_assert_held(&inode->i_rwsem);