[Cluster-devel] [PATCH] gfs2: free disk inode which is deleted by remote node

[Cluster-devel] [PATCH] gfs2: free disk inode which is deleted by remote node

[Wengang Wang]

this patch is for the same problem that Benjamin Marzinski fixes at commit
b94a170e96dc416828af9d350ae2e34b70ae7347

quotation of the original problem:

---cut here---
When a file is deleted from a gfs2 filesystem on one node, a dcache
entry for it may still exist on other nodes in the cluster. If this
happens, gfs2 will be unable to free this file on disk. Because of this,
it's possible to have a gfs2 filesystem with no files on it and no free
space. With this patch, when a node receives a callback notifying it
that the file is being deleted on another node, it schedules a new
workqueue thread to remove the file's dcache entry.
---end cut---

after applying Benjamin's patch, I think there is still a case in which the disk
inode remains even when "no space" is hit. the case is that when running
d_prune_aliases() against the inode, there are one or more dentries(aliases)
which have reference count number > 0. in this case the dentries won't be pruned.
and even later, the reference cound becomes to 0, the dentries can still be
cached in memory. unfortunately, no callback come again, things come back to
the state before the callback runs. thus the on disk inode remains there until
in memory inode is removed for some other reason(shrinking inode cache or unmount
the gfs2 volume..).

this patch is to remove those dentries when their reference count becomes to 0 and
the inode is deleted by remote node. for implementation, gfs2_dentry_delete() is
added as dentry_operations.d_delete. the function returns true when the inode is
deleted by remote node. in dput(), gfs2_dentry_delete() is called and since it
returns true, the dentry is unhashed from dcache and then removed. when all dentries
are removed, the in memory inode get removed so that the on disk inode can be freed.

Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
---
 fs/gfs2/dentry.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
index 022c66c..434f204 100644
--- a/fs/gfs2/dentry.c
+++ b/fs/gfs2/dentry.c
@@ -107,8 +107,19 @@ static int gfs2_dhash(struct dentry *dentry, struct qstr *str)
 	return 0;
 }
 
+static int gfs2_dentry_delete(struct dentry *dentry)
+{
+	struct gfs2_inode *ginode = GFS2_I(dentry->d_inode);
+
+	if (test_bit(GLF_DEMOTE, &ginode->i_iopen_gh.gh_gl->gl_flags))
+		return 1;
+
+	return 0;
+}
+
 const struct dentry_operations gfs2_dops = {
 	.d_revalidate = gfs2_drevalidate,
 	.d_hash = gfs2_dhash,
+	.d_delete = gfs2_dentry_delete,
 };
 
-- 
1.6.2.5

[Cluster-devel] [PATCH] gfs2: free disk inode which is deleted by remote node

[Steven Whitehouse]

Hi,

Don't worry about the wrong list, I did spot the patch anyway. I agree
with the principle of what you are trying to do, but something isn't
quite right. When I tried this on Friday I got a NULL pointer deref the
first time I tried to touch a file on the fs.

I think it needs a bit more work, but its certainly going in the right
direction,

Steve.

On Fri, 2009-08-14 at 23:05 +0800, Wengang Wang wrote:
> this patch is for the same problem that Benjamin Marzinski fixes at commit
> b94a170e96dc416828af9d350ae2e34b70ae7347
> 
> quotation of the original problem:
> 
> ---cut here---
> When a file is deleted from a gfs2 filesystem on one node, a dcache
> entry for it may still exist on other nodes in the cluster. If this
> happens, gfs2 will be unable to free this file on disk. Because of this,
> it's possible to have a gfs2 filesystem with no files on it and no free
> space. With this patch, when a node receives a callback notifying it
> that the file is being deleted on another node, it schedules a new
> workqueue thread to remove the file's dcache entry.
> ---end cut---
> 
> after applying Benjamin's patch, I think there is still a case in which the disk
> inode remains even when "no space" is hit. the case is that when running
> d_prune_aliases() against the inode, there are one or more dentries(aliases)
> which have reference count number > 0. in this case the dentries won't be pruned.
> and even later, the reference cound becomes to 0, the dentries can still be
> cached in memory. unfortunately, no callback come again, things come back to
> the state before the callback runs. thus the on disk inode remains there until
> in memory inode is removed for some other reason(shrinking inode cache or unmount
> the gfs2 volume..).
> 
> this patch is to remove those dentries when their reference count becomes to 0 and
> the inode is deleted by remote node. for implementation, gfs2_dentry_delete() is
> added as dentry_operations.d_delete. the function returns true when the inode is
> deleted by remote node. in dput(), gfs2_dentry_delete() is called and since it
> returns true, the dentry is unhashed from dcache and then removed. when all dentries
> are removed, the in memory inode get removed so that the on disk inode can be freed.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
> ---
>  fs/gfs2/dentry.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/gfs2/dentry.c b/fs/gfs2/dentry.c
> index 022c66c..434f204 100644
> --- a/fs/gfs2/dentry.c
> +++ b/fs/gfs2/dentry.c
> @@ -107,8 +107,19 @@ static int gfs2_dhash(struct dentry *dentry, struct qstr *str)
>  	return 0;
>  }
>  
> +static int gfs2_dentry_delete(struct dentry *dentry)
> +{
> +	struct gfs2_inode *ginode = GFS2_I(dentry->d_inode);
> +
> +	if (test_bit(GLF_DEMOTE, &ginode->i_iopen_gh.gh_gl->gl_flags))
> +		return 1;
> +
> +	return 0;
> +}
> +
>  const struct dentry_operations gfs2_dops = {
>  	.d_revalidate = gfs2_drevalidate,
>  	.d_hash = gfs2_dhash,
> +	.d_delete = gfs2_dentry_delete,
>  };
>  

[Cluster-devel] [PATCH] gfs2: free disk inode which is deleted by remote node

[Wengang Wang]

Hi Steve,

Steven Whitehouse wrote:
> Hi,
> 
> Don't worry about the wrong list, I did spot the patch anyway. I agree
> with the principle of what you are trying to do, but something isn't
> quite right. When I tried this on Friday I got a NULL pointer deref the
> first time I tried to touch a file on the fs.
> 
> I think it needs a bit more work, but its certainly going in the right
> direction,

encouraged :)
and yes the dentry->d_inode is set to NULL in path 
vfs_unlink()->d_delete()->dentry_iput(). I didn't test it out since I 
wasn't running on the patched code on the other node, sorry for that.
I'm going to send the v2 version of the patch soon.
if you are going to commit the patch, please help(and feel free) to 
correct mistakes, it there are, in my patch log --my English is not good.

regards,
wengang.