Re: [PATCH 2/5] btrfs: Split btrfs_del_delalloc_inode into 2 functions

[David Sterba]

On Fri, May 11, 2018 at 11:39:56AM +0300, Nikolay Borisov wrote:
> On 11.05.2018 08:44, Anand Jain wrote:
> > On 04/27/2018 05:21 PM, Nikolay Borisov wrote:
> >> This is in preparation of fixing delalloc inodes leakage on transaction
> >> abort. Also export the new function.
> >>
> >> Signed-off-by: Nikolay Borisov 
> > 
> >  nit: I think we are reserving function prefix __ for some special
> >  functions. I am not sure if the function name should prefix with __
> >  here.
> 
> Generally __ prefix is used for some internal function. In this case the
> gist of the function (with no locking) is behind the __ prefixed
> function, whereas the non __ version adds the necessary locking. I think
> this is a fairly well-established pattern in the kernel.

Yes, this is one of few patterns where __ is ok.
--
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

Re: [PATCH 2/5] btrfs: Split btrfs_del_delalloc_inode into 2 functions

[Nikolay Borisov]

On 11.05.2018 08:44, Anand Jain wrote:
> 
> 
> On 04/27/2018 05:21 PM, Nikolay Borisov wrote:
>> This is in preparation of fixing delalloc inodes leakage on transaction
>> abort. Also export the new function.
>>
>> Signed-off-by: Nikolay Borisov 
> 
>  nit: I think we are reserving function prefix __ for some special
>  functions. I am not sure if the function name should prefix with __
>  here.

Generally __ prefix is used for some internal function. In this case the
gist of the function (with no locking) is behind the __ prefixed
function, whereas the non __ version adds the necessary locking. I think
this is a fairly well-established pattern in the kernel.
> 
> Reviewed-by: Anand Jain 
> 
> Thanks, Anand
> -- 
> 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
> 
--
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

Re: [PATCH 2/5] btrfs: Split btrfs_del_delalloc_inode into 2 functions

[Anand Jain]

On 04/27/2018 05:21 PM, Nikolay Borisov wrote:

This is in preparation of fixing delalloc inodes leakage on transaction
abort. Also export the new function.

Signed-off-by: Nikolay Borisov 


 nit: I think we are reserving function prefix __ for some special
 functions. I am not sure if the function name should prefix with __
 here.

Reviewed-by: Anand Jain 

Thanks, Anand
--
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 2/5] btrfs: Split btrfs_del_delalloc_inode into 2 functions

[Nikolay Borisov]

This is in preparation of fixing delalloc inodes leakage on transaction
abort. Also export the new function.

Signed-off-by: Nikolay Borisov 
---
 fs/btrfs/ctree.h |  2 ++
 fs/btrfs/inode.c | 15 ---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5b7080d2fbff..27aa9b58b001 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3174,6 +3174,8 @@ noinline int can_nocow_extent(struct inode *inode, u64 
offset, u64 *len,
  u64 *orig_start, u64 *orig_block_len,
  u64 *ram_bytes);
 
+void __btrfs_del_delalloc_inode(struct btrfs_root *root,
+   struct btrfs_inode *inode);
 struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry);
 int btrfs_set_inode_index(struct btrfs_inode *dir, u64 *index);
 int btrfs_unlink_inode(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 078e02c21825..164950d96c8a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1742,12 +1742,12 @@ static void btrfs_add_delalloc_inodes(struct btrfs_root 
*root,
spin_unlock(>delalloc_lock);
 }
 
-static void btrfs_del_delalloc_inode(struct btrfs_root *root,
-struct btrfs_inode *inode)
+
+void __btrfs_del_delalloc_inode(struct btrfs_root *root,
+   struct btrfs_inode *inode)
 {
struct btrfs_fs_info *fs_info = btrfs_sb(inode->vfs_inode.i_sb);
 
-   spin_lock(>delalloc_lock);
if (!list_empty(>delalloc_inodes)) {
list_del_init(>delalloc_inodes);
clear_bit(BTRFS_INODE_IN_DELALLOC_LIST,
@@ -1760,6 +1760,15 @@ static void btrfs_del_delalloc_inode(struct btrfs_root 
*root,
spin_unlock(_info->delalloc_root_lock);
}
}
+}
+
+
+static void btrfs_del_delalloc_inode(struct btrfs_root *root,
+struct btrfs_inode *inode)
+{
+
+   spin_lock(>delalloc_lock);
+   __btrfs_del_delalloc_inode(root, inode);
spin_unlock(>delalloc_lock);
 }
 
-- 
2.7.4

--
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