Re: [PATCH] btrfs: cleanup extent locking sequence

[David Sterba]

On Tue, Oct 10, 2017 at 09:26:41PM -0500, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues 
> 
> Code cleanup for better understanding:
> needs_unlock to be called extent_locked to show state as opposed to
> action.
> Changed the variable to int, to reduce code in the critical path(code usually
> executed).
> 
> Signed-off-by: Goldwyn Rodrigues 
> ---
>  fs/btrfs/file.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index f6c6754cf52d..a3d006d14683 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1590,7 +1590,7 @@ static noinline ssize_t __btrfs_buffered_write(struct 
> file *file,
>   int ret = 0;
>   bool only_release_metadata = false;
>   bool force_page_uptodate = false;
> - bool need_unlock;
> + int extents_locked;

The variable can be moved to the while() { } block

>  
>   nrptrs = min(DIV_ROUND_UP(iov_iter_count(i), PAGE_SIZE),
>   PAGE_SIZE / (sizeof(struct page *)));
> @@ -1670,7 +1670,6 @@ static noinline ssize_t __btrfs_buffered_write(struct 
> file *file,
>   }
>  
>   release_bytes = reserve_bytes;
> - need_unlock = false;
>  again:
>   /*
>* This is going to setup the pages array with the number of
> @@ -1683,16 +1682,15 @@ static noinline ssize_t __btrfs_buffered_write(struct 
> file *file,
>   if (ret)
>   break;
>  
> - ret = lock_and_cleanup_extent_if_need(BTRFS_I(inode), pages,
> + extents_locked = lock_and_cleanup_extent_if_need(
> + BTRFS_I(inode), pages,
>   num_pages, pos, write_bytes, ,
>   , _state);
> - if (ret < 0) {
> + if (extents_locked < 0) {
>   if (ret == -EAGAIN)

either

if (extents_locked == -EAGAIN)

or

move 'ret = extents_locked' above the if.

>   goto again;
> + ret = extents_locked;
>   break;
> - } else if (ret > 0) {
> - need_unlock = true;
> - ret = 0;
>   }
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] btrfs: cleanup extent locking sequence

[Goldwyn Rodrigues]

From: Goldwyn Rodrigues 

Code cleanup for better understanding:
needs_unlock to be called extent_locked to show state as opposed to
action.
Changed the variable to int, to reduce code in the critical path(code usually
executed).

Signed-off-by: Goldwyn Rodrigues 
---
 fs/btrfs/file.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index f6c6754cf52d..a3d006d14683 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1590,7 +1590,7 @@ static noinline ssize_t __btrfs_buffered_write(struct 
file *file,
int ret = 0;
bool only_release_metadata = false;
bool force_page_uptodate = false;
-   bool need_unlock;
+   int extents_locked;
 
nrptrs = min(DIV_ROUND_UP(iov_iter_count(i), PAGE_SIZE),
PAGE_SIZE / (sizeof(struct page *)));
@@ -1670,7 +1670,6 @@ static noinline ssize_t __btrfs_buffered_write(struct 
file *file,
}
 
release_bytes = reserve_bytes;
-   need_unlock = false;
 again:
/*
 * This is going to setup the pages array with the number of
@@ -1683,16 +1682,15 @@ static noinline ssize_t __btrfs_buffered_write(struct 
file *file,
if (ret)
break;
 
-   ret = lock_and_cleanup_extent_if_need(BTRFS_I(inode), pages,
+   extents_locked = lock_and_cleanup_extent_if_need(
+   BTRFS_I(inode), pages,
num_pages, pos, write_bytes, ,
, _state);
-   if (ret < 0) {
+   if (extents_locked < 0) {
if (ret == -EAGAIN)
goto again;
+   ret = extents_locked;
break;
-   } else if (ret > 0) {
-   need_unlock = true;
-   ret = 0;
}
 
copied = btrfs_copy_from_user(pos, write_bytes, pages, i);
@@ -1754,7 +1752,7 @@ static noinline ssize_t __btrfs_buffered_write(struct 
file *file,
if (copied > 0)
ret = btrfs_dirty_pages(inode, pages, dirty_pages,
pos, copied, NULL);
-   if (need_unlock)
+   if (extents_locked)
unlock_extent_cached(_I(inode)->io_tree,
 lockstart, lockend, _state,
 GFP_NOFS);
-- 
2.14.2

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html