All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
@ 2018-02-26  9:54 Nikolay Borisov
  2018-03-03  0:24 ` Andrew Morton
  0 siblings, 1 reply; 7+ messages in thread
From: Nikolay Borisov @ 2018-02-26  9:54 UTC (permalink / raw)
  To: akpm; +Cc: linux-fsdevel, linux-kernel, viro, Nikolay Borisov

We already get the block counts and the calculate the end block at the
beginning of the function. Let's use the local variables for consistency and
readability. No functional changes

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
---
Andrew, 

Sending to you since this has been languishing on the mailing for quite some 
time. The patch has been reviewed here:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1460544.html

fs/direct-io.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index a8131087aa1c..a73448a501c2 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1200,7 +1200,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 	}
 
 	/* watch out for a 0 len io from a tricksy fs */
-	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
+	if (iov_iter_rw(iter) == READ && !count)
 		return 0;
 
 	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
@@ -1316,8 +1316,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 
 	dio->should_dirty = (iter->type == ITER_IOVEC);
 	sdio.iter = iter;
-	sdio.final_block_in_request =
-		(offset + iov_iter_count(iter)) >> blkbits;
+	sdio.final_block_in_request = end >> blkbits;
 
 	/*
 	 * In case of non-aligned buffers, we may need 2 more
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
  2018-02-26  9:54 [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO Nikolay Borisov
@ 2018-03-03  0:24 ` Andrew Morton
  2018-03-03  8:16   ` Nikolay Borisov
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2018-03-03  0:24 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-fsdevel, linux-kernel, viro

On Mon, 26 Feb 2018 11:54:30 +0200 Nikolay Borisov <nborisov@suse.com> wrote:

> We already get the block counts and the calculate the end block at the
> beginning of the function. Let's use the local variables for consistency and
> readability. No functional changes
> 

Fair enough.

> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -1200,7 +1200,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  	}
>  
>  	/* watch out for a 0 len io from a tricksy fs */
> -	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
> +	if (iov_iter_rw(iter) == READ && !count)
>  		return 0;
>  
>  	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
> @@ -1316,8 +1316,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  
>  	dio->should_dirty = (iter->type == ITER_IOVEC);
>  	sdio.iter = iter;
> -	sdio.final_block_in_request =
> -		(offset + iov_iter_count(iter)) >> blkbits;
> +	sdio.final_block_in_request = end >> blkbits;
>  
>  	/*
>  	 * In case of non-aligned buffers, we may need 2 more

It's always scary to see a local being used in this fashion so far away
from where it was initialized.  However, we can get our warm fuzzy
feelings by doing this:

--- a/fs/direct-io.c~direct-io-minor-cleanups-in-do_blockdev_direct_io-fix
+++ a/fs/direct-io.c
@@ -1178,9 +1178,9 @@ do_blockdev_direct_IO(struct kiocb *iocb
 	unsigned blkbits = i_blkbits;
 	unsigned blocksize_mask = (1 << blkbits) - 1;
 	ssize_t retval = -EINVAL;
-	size_t count = iov_iter_count(iter);
+	const size_t count = iov_iter_count(iter);
 	loff_t offset = iocb->ki_pos;
-	loff_t end = offset + count;
+	const loff_t end = offset + count;
 	struct dio *dio;
 	struct dio_submit sdio = { 0, };
 	struct buffer_head map_bh = { 0, };
_

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
  2018-03-03  0:24 ` Andrew Morton
@ 2018-03-03  8:16   ` Nikolay Borisov
  0 siblings, 0 replies; 7+ messages in thread
From: Nikolay Borisov @ 2018-03-03  8:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, viro



On  3.03.2018 02:24, Andrew Morton wrote:
> On Mon, 26 Feb 2018 11:54:30 +0200 Nikolay Borisov <nborisov@suse.com> wrote:
> 
>> We already get the block counts and the calculate the end block at the
>> beginning of the function. Let's use the local variables for consistency and
>> readability. No functional changes
>>
> 
> Fair enough.
> 
>> --- a/fs/direct-io.c
>> +++ b/fs/direct-io.c
>> @@ -1200,7 +1200,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>>  	}
>>  
>>  	/* watch out for a 0 len io from a tricksy fs */
>> -	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
>> +	if (iov_iter_rw(iter) == READ && !count)
>>  		return 0;
>>  
>>  	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
>> @@ -1316,8 +1316,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>>  
>>  	dio->should_dirty = (iter->type == ITER_IOVEC);
>>  	sdio.iter = iter;
>> -	sdio.final_block_in_request =
>> -		(offset + iov_iter_count(iter)) >> blkbits;
>> +	sdio.final_block_in_request = end >> blkbits;
>>  
>>  	/*
>>  	 * In case of non-aligned buffers, we may need 2 more
> 
> It's always scary to see a local being used in this fashion so far away
> from where it was initialized.  However, we can get our warm fuzzy
> feelings by doing this:
> 

Good point !

> --- a/fs/direct-io.c~direct-io-minor-cleanups-in-do_blockdev_direct_io-fix
> +++ a/fs/direct-io.c
> @@ -1178,9 +1178,9 @@ do_blockdev_direct_IO(struct kiocb *iocb
>  	unsigned blkbits = i_blkbits;
>  	unsigned blocksize_mask = (1 << blkbits) - 1;
>  	ssize_t retval = -EINVAL;
> -	size_t count = iov_iter_count(iter);
> +	const size_t count = iov_iter_count(iter);
>  	loff_t offset = iocb->ki_pos;
> -	loff_t end = offset + count;
> +	const loff_t end = offset + count;
>  	struct dio *dio;
>  	struct dio_submit sdio = { 0, };
>  	struct buffer_head map_bh = { 0, };
> _

I assume you don't want me to resend but will just fold the changes
yourself?

> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
  2017-08-02  7:10 Nikolay Borisov
  2017-08-03 14:08 ` Jeff Moyer
  2017-09-15 19:34 ` Nikolay Borisov
@ 2018-02-21 17:48 ` Nikolay Borisov
  2 siblings, 0 replies; 7+ messages in thread
From: Nikolay Borisov @ 2018-02-21 17:48 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-kernel



On  2.08.2017 10:10, Nikolay Borisov wrote:
> We already get the block counts and the calculate the end block at the
> beginning of the function. Let's use the local variables for consistency and
> readability. No functional changes
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/direct-io.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 08cf27811e5a..987bc17a5f5e 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -1139,7 +1139,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  	}
>  
>  	/* watch out for a 0 len io from a tricksy fs */
> -	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
> +	if (iov_iter_rw(iter) == READ && !count)
>  		return 0;
>  
>  	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
> @@ -1248,8 +1248,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  
>  	dio->should_dirty = (iter->type == ITER_IOVEC);
>  	sdio.iter = iter;
> -	sdio.final_block_in_request =
> -		(offset + iov_iter_count(iter)) >> blkbits;
> +	sdio.final_block_in_request = end >> blkbits;
>  
>  	/*
>  	 * In case of non-aligned buffers, we may need 2 more
> 


Ping on that forgotten one :)

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
  2017-08-02  7:10 Nikolay Borisov
  2017-08-03 14:08 ` Jeff Moyer
@ 2017-09-15 19:34 ` Nikolay Borisov
  2018-02-21 17:48 ` Nikolay Borisov
  2 siblings, 0 replies; 7+ messages in thread
From: Nikolay Borisov @ 2017-09-15 19:34 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-kernel



On  2.08.2017 10:10, Nikolay Borisov wrote:
> We already get the block counts and the calculate the end block at the
> beginning of the function. Let's use the local variables for consistency and
> readability. No functional changes
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---

ping

>  fs/direct-io.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 08cf27811e5a..987bc17a5f5e 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -1139,7 +1139,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  	}
>  
>  	/* watch out for a 0 len io from a tricksy fs */
> -	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
> +	if (iov_iter_rw(iter) == READ && !count)
>  		return 0;
>  
>  	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
> @@ -1248,8 +1248,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
>  
>  	dio->should_dirty = (iter->type == ITER_IOVEC);
>  	sdio.iter = iter;
> -	sdio.final_block_in_request =
> -		(offset + iov_iter_count(iter)) >> blkbits;
> +	sdio.final_block_in_request = end >> blkbits;
>  
>  	/*
>  	 * In case of non-aligned buffers, we may need 2 more
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
  2017-08-02  7:10 Nikolay Borisov
@ 2017-08-03 14:08 ` Jeff Moyer
  2017-09-15 19:34 ` Nikolay Borisov
  2018-02-21 17:48 ` Nikolay Borisov
  2 siblings, 0 replies; 7+ messages in thread
From: Jeff Moyer @ 2017-08-03 14:08 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: viro, linux-fsdevel, linux-kernel

Nikolay Borisov <nborisov@suse.com> writes:

> We already get the block counts and the calculate the end block at the
> beginning of the function. Let's use the local variables for consistency and
> readability. No functional changes
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Looks ok to me.

Reviewed-by: Jeff Moyer <jmoyer@redhat.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO
@ 2017-08-02  7:10 Nikolay Borisov
  2017-08-03 14:08 ` Jeff Moyer
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Nikolay Borisov @ 2017-08-02  7:10 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-kernel, Nikolay Borisov

We already get the block counts and the calculate the end block at the
beginning of the function. Let's use the local variables for consistency and
readability. No functional changes

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/direct-io.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 08cf27811e5a..987bc17a5f5e 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1139,7 +1139,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 	}
 
 	/* watch out for a 0 len io from a tricksy fs */
-	if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
+	if (iov_iter_rw(iter) == READ && !count)
 		return 0;
 
 	dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
@@ -1248,8 +1248,7 @@ do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
 
 	dio->should_dirty = (iter->type == ITER_IOVEC);
 	sdio.iter = iter;
-	sdio.final_block_in_request =
-		(offset + iov_iter_count(iter)) >> blkbits;
+	sdio.final_block_in_request = end >> blkbits;
 
 	/*
 	 * In case of non-aligned buffers, we may need 2 more
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-03  8:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-26  9:54 [PATCH] direct-io: Minor cleanups in do_blockdev_direct_IO Nikolay Borisov
2018-03-03  0:24 ` Andrew Morton
2018-03-03  8:16   ` Nikolay Borisov
  -- strict thread matches above, loose matches on Subject: below --
2017-08-02  7:10 Nikolay Borisov
2017-08-03 14:08 ` Jeff Moyer
2017-09-15 19:34 ` Nikolay Borisov
2018-02-21 17:48 ` Nikolay Borisov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.