[PATCH 0/2] Reenable ceph dcache usage

[PATCH 0/2] Reenable ceph dcache usage

[Sage Weil]

Hi Al, Christoph,

These two patches export d_find_any_alias() and reenable the Ceph use of 
the dcache for negative lookups and readdir without hitting the server.  
The behavior is controllable via a mount option (and now documented).

Unless there are any objections to the export, I plan to send this to 
Linus shortly.

Thanks!
sage


Sage Weil (2):
  vfs: export symbol d_find_any_alias()
  ceph: enable/disable dentry complete flags via mount option

 Documentation/filesystems/ceph.txt |   16 +++++++++++-----
 fs/ceph/dir.c                      |   25 ++++++++++++++++++++++---
 fs/ceph/super.c                    |   14 ++++++++++++++
 fs/ceph/super.h                    |    1 +
 fs/dcache.c                        |    4 ++--
 include/linux/dcache.h             |    1 +
 6 files changed, 51 insertions(+), 10 deletions(-)

[PATCH 1/2] vfs: export symbol d_find_any_alias()

[Sage Weil]

Ceph needs this.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/dcache.c            |    4 ++--
 include/linux/dcache.h |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 89509b5..80208e6 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1471,7 +1471,7 @@ static struct dentry * __d_find_any_alias(struct inode *inode)
 	return alias;
 }
 
-static struct dentry * d_find_any_alias(struct inode *inode)
+struct dentry * d_find_any_alias(struct inode *inode)
 {
 	struct dentry *de;
 
@@ -1480,7 +1480,7 @@ static struct dentry * d_find_any_alias(struct inode *inode)
 	spin_unlock(&inode->i_lock);
 	return de;
 }
-
+EXPORT_SYMBOL(d_find_any_alias);
 
 /**
  * d_obtain_alias - find or allocate a dentry for a given inode
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index ed9f74f..86079df 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -241,6 +241,7 @@ extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
 extern struct dentry * d_alloc_pseudo(struct super_block *, const struct qstr *);
 extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
 extern struct dentry * d_add_ci(struct dentry *, struct inode *, struct qstr *);
+extern struct dentry * d_find_any_alias(struct inode *inode);
 extern struct dentry * d_obtain_alias(struct inode *);
 extern void shrink_dcache_sb(struct super_block *);
 extern void shrink_dcache_parent(struct dentry *);
-- 
1.7.0

Re: [PATCH 1/2] vfs: export symbol d_find_any_alias()

[Christoph Hellwig]

On Wed, Jan 11, 2012 at 10:46:41AM -0800, Sage Weil wrote:
> Ceph needs this.
> 
> Signed-off-by: Sage Weil <sage@newdream.net>

Can you add a kerneldoc comment now that it is exported?

> -static struct dentry * d_find_any_alias(struct inode *inode)
> +struct dentry * d_find_any_alias(struct inode *inode)

also if you touch the line anyway please remove the superflous
whitespace after the '*'.

Re: [PATCH 1/2] vfs: export symbol d_find_any_alias()

[Sage Weil]

On Wed, 11 Jan 2012, Christoph Hellwig wrote:
> On Wed, Jan 11, 2012 at 10:46:41AM -0800, Sage Weil wrote:
> > Ceph needs this.
> > 
> > Signed-off-by: Sage Weil <sage@newdream.net>
> 
> Can you add a kerneldoc comment now that it is exported?
> 
> > -static struct dentry * d_find_any_alias(struct inode *inode)
> > +struct dentry * d_find_any_alias(struct inode *inode)
> 
> also if you touch the line anyway please remove the superflous
> whitespace after the '*'.

I almost did that, until the prototype stuck out like a sore thumb in 
dcache.h.  Fixed up, see below:


>From d0207b0a2646a20e25ca8729a1d18ee74fdabfb9 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@newdream.net>
Date: Tue, 10 Jan 2012 09:04:37 -0800
Subject: [PATCH 1/2] vfs: export symbol d_find_any_alias()

Ceph needs this.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/dcache.c            |   11 +++++++++--
 include/linux/dcache.h |    1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 89509b5..ba96005 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1471,7 +1471,14 @@ static struct dentry * __d_find_any_alias(struct inode *inode)
 	return alias;
 }
 
-static struct dentry * d_find_any_alias(struct inode *inode)
+/**
+ * d_find_any_alias - find any alias for a given inode
+ * @inode: inode to find an alias for
+ *
+ * If any aliases exist for the given inode, take and return a
+ * reference for one of them.  If no aliases exist, return %NULL.
+ */
+struct dentry *d_find_any_alias(struct inode *inode)
 {
 	struct dentry *de;
 
@@ -1480,7 +1487,7 @@ static struct dentry * d_find_any_alias(struct inode *inode)
 	spin_unlock(&inode->i_lock);
 	return de;
 }
-
+EXPORT_SYMBOL(d_find_any_alias);
 
 /**
  * d_obtain_alias - find or allocate a dentry for a given inode
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index ed9f74f..3871ba7 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -241,6 +241,7 @@ extern struct dentry * d_alloc(struct dentry *, const struct qstr *);
 extern struct dentry * d_alloc_pseudo(struct super_block *, const struct qstr *);
 extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
 extern struct dentry * d_add_ci(struct dentry *, struct inode *, struct qstr *);
+extern struct dentry *d_find_any_alias(struct inode *inode);
 extern struct dentry * d_obtain_alias(struct inode *);
 extern void shrink_dcache_sb(struct super_block *);
 extern void shrink_dcache_parent(struct dentry *);
-- 
1.7.0

Re: [PATCH 1/2] vfs: export symbol d_find_any_alias()

[Christoph Hellwig]

> >From d0207b0a2646a20e25ca8729a1d18ee74fdabfb9 Mon Sep 17 00:00:00 2001
> From: Sage Weil <sage@newdream.net>
> Date: Tue, 10 Jan 2012 09:04:37 -0800
> Subject: [PATCH 1/2] vfs: export symbol d_find_any_alias()
> 
> Ceph needs this.
> 
> Signed-off-by: Sage Weil <sage@newdream.net>

Looks good,


Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/2] ceph: enable/disable dentry complete flags via mount option

[Sage Weil]

Enable/disable use of the dentry dir 'complete' flag via a mount option.
This lets the admin control whether ceph uses the dcache to satisfy
negative lookups or readdir when it has the entire directory contents in
its cache.

This is purely a performance optimization; correctness is guaranteed
whether it is enabled or not.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 Documentation/filesystems/ceph.txt |   16 +++++++++++-----
 fs/ceph/dir.c                      |   25 ++++++++++++++++++++++---
 fs/ceph/super.c                    |   14 ++++++++++++++
 fs/ceph/super.h                    |    1 +
 4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/ceph.txt b/Documentation/filesystems/ceph.txt
index 763d8eb..36af18e 100644
--- a/Documentation/filesystems/ceph.txt
+++ b/Documentation/filesystems/ceph.txt
@@ -119,12 +119,18 @@ Mount Options
 	must rely on TCP's error correction to detect data corruption
 	in the data payload.
 
-  noasyncreaddir
-	Disable client's use its local cache to satisfy	readdir
-	requests.  (This does not change correctness; the client uses
-	cached metadata only when a lease or capability ensures it is
-	valid.)
+  dcache
+        Use the dcache contents to perform negative lookups and
+        readdir when the client has the entire directory contents in
+        its cache.  (This does not change correctness; the client uses
+        cached metadata only when a lease or capability ensures it is
+        valid.)
+
+  nodcache
+        Do not use the dcache as above.
 
+  noasyncreaddir
+	Do not use the dcache as above for readdir.
 
 More Information
 ================
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b230b8f..ab2b035 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1104,17 +1104,36 @@ static int ceph_snapdir_d_revalidate(struct dentry *dentry,
  */
 void ceph_dir_set_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+	
+	if (dentry && ceph_dentry(dentry) &&
+	    ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), DCACHE)) {
+		dout(" marking %p (%p) complete\n", inode, dentry);
+		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 }
 
 void ceph_dir_clear_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+
+	if (dentry && ceph_dentry(dentry)) {
+		dout(" marking %p (%p) complete\n", inode, dentry);
+		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 }
 
 bool ceph_dir_test_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+
+	if (dentry && ceph_dentry(dentry)) {
+		dout(" marking %p (%p) NOT complete\n", inode, dentry);
+		clear_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 	return false;
 }
 
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index ec74313..9c62fe0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -131,6 +131,8 @@ enum {
 	Opt_rbytes,
 	Opt_norbytes,
 	Opt_noasyncreaddir,
+	Opt_dcache,
+	Opt_nodcache,
 	Opt_ino32,
 };
 
@@ -152,6 +154,8 @@ static match_table_t fsopt_tokens = {
 	{Opt_rbytes, "rbytes"},
 	{Opt_norbytes, "norbytes"},
 	{Opt_noasyncreaddir, "noasyncreaddir"},
+	{Opt_dcache, "dcache"},
+	{Opt_nodcache, "nodcache"},
 	{Opt_ino32, "ino32"},
 	{-1, NULL}
 };
@@ -231,6 +235,12 @@ static int parse_fsopt_token(char *c, void *private)
 	case Opt_noasyncreaddir:
 		fsopt->flags |= CEPH_MOUNT_OPT_NOASYNCREADDIR;
 		break;
+	case Opt_dcache:
+		fsopt->flags |= CEPH_MOUNT_OPT_DCACHE;
+		break;
+	case Opt_nodcache:
+		fsopt->flags &= ~CEPH_MOUNT_OPT_DCACHE;
+		break;
 	case Opt_ino32:
 		fsopt->flags |= CEPH_MOUNT_OPT_INO32;
 		break;
@@ -377,6 +387,10 @@ static int ceph_show_options(struct seq_file *m, struct vfsmount *mnt)
 		seq_puts(m, ",norbytes");
 	if (fsopt->flags & CEPH_MOUNT_OPT_NOASYNCREADDIR)
 		seq_puts(m, ",noasyncreaddir");
+	if (fsopt->flags & CEPH_MOUNT_OPT_DCACHE)
+		seq_puts(m, ",dcache");
+	else
+		seq_puts(m, ",nodcache");
 
 	if (fsopt->wsize)
 		seq_printf(m, ",wsize=%d", fsopt->wsize);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index edcbf37..140f99f 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -28,6 +28,7 @@
 #define CEPH_MOUNT_OPT_RBYTES          (1<<5) /* dir st_bytes = rbytes */
 #define CEPH_MOUNT_OPT_NOASYNCREADDIR  (1<<7) /* no dcache readdir */
 #define CEPH_MOUNT_OPT_INO32           (1<<8) /* 32 bit inos */
+#define CEPH_MOUNT_OPT_DCACHE          (1<<9) /* use dcache for readdir etc */
 
 #define CEPH_MOUNT_OPT_DEFAULT    (CEPH_MOUNT_OPT_RBYTES)
 
-- 
1.7.0

Re: [PATCH 2/2] ceph: enable/disable dentry complete flags via mount option

[Christoph Hellwig]

> +  dcache
> +        Use the dcache contents to perform negative lookups and
> +        readdir when the client has the entire directory contents in
> +        its cache.  (This does not change correctness; the client uses
> +        cached metadata only when a lease or capability ensures it is
> +        valid.)
> +
> +  nodcache
> +        Do not use the dcache as above.
>  
> +  noasyncreaddir
> +	Do not use the dcache as above for readdir.

None of thie explains why you'd ever want to turn the flag off.

Re: [PATCH 2/2] ceph: enable/disable dentry complete flags via mount option

[Sage Weil]

On Wed, 11 Jan 2012, Christoph Hellwig wrote:
> > +  dcache
> > +        Use the dcache contents to perform negative lookups and
> > +        readdir when the client has the entire directory contents in
> > +        its cache.  (This does not change correctness; the client uses
> > +        cached metadata only when a lease or capability ensures it is
> > +        valid.)
> > +
> > +  nodcache
> > +        Do not use the dcache as above.
> >  
> > +  noasyncreaddir
> > +	Do not use the dcache as above for readdir.
> 
> None of thie explains why you'd ever want to turn the flag off.

See below:

>From 755052c0867755c0acb12de4878e29da9301e4f3 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@newdream.net>
Date: Tue, 10 Jan 2012 09:12:55 -0800
Subject: [PATCH 2/2] ceph: enable/disable dentry complete flags via mount option

Enable/disable use of the dentry dir 'complete' flag via a mount option.
This lets the admin control whether ceph uses the dcache to satisfy
negative lookups or readdir when it has the entire directory contents in
its cache.

This is purely a performance optimization; correctness is guaranteed
whether it is enabled or not.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 Documentation/filesystems/ceph.txt |   18 +++++++++++++-----
 fs/ceph/dir.c                      |   25 ++++++++++++++++++++++---
 fs/ceph/super.c                    |   14 ++++++++++++++
 fs/ceph/super.h                    |    1 +
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/Documentation/filesystems/ceph.txt b/Documentation/filesystems/ceph.txt
index 763d8eb..d6030aa 100644
--- a/Documentation/filesystems/ceph.txt
+++ b/Documentation/filesystems/ceph.txt
@@ -119,12 +119,20 @@ Mount Options
 	must rely on TCP's error correction to detect data corruption
 	in the data payload.
 
-  noasyncreaddir
-	Disable client's use its local cache to satisfy	readdir
-	requests.  (This does not change correctness; the client uses
-	cached metadata only when a lease or capability ensures it is
-	valid.)
+  dcache
+        Use the dcache contents to perform negative lookups and
+        readdir when the client has the entire directory contents in
+        its cache.  (This does not change correctness; the client uses
+        cached metadata only when a lease or capability ensures it is
+        valid.)
+
+  nodcache
+        Do not use the dcache as above.  This avoids a significant amount of
+        complex code, sacrificing performance without affecting correctness,
+        and is useful for tracking down bugs.
 
+  noasyncreaddir
+	Do not use the dcache as above for readdir.
 
 More Information
 ================
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b230b8f..ab2b035 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1104,17 +1104,36 @@ static int ceph_snapdir_d_revalidate(struct dentry *dentry,
  */
 void ceph_dir_set_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+	
+	if (dentry && ceph_dentry(dentry) &&
+	    ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), DCACHE)) {
+		dout(" marking %p (%p) complete\n", inode, dentry);
+		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 }
 
 void ceph_dir_clear_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+
+	if (dentry && ceph_dentry(dentry)) {
+		dout(" marking %p (%p) complete\n", inode, dentry);
+		set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 }
 
 bool ceph_dir_test_complete(struct inode *inode)
 {
-	/* not yet implemented */
+	struct dentry *dentry = d_find_any_alias(inode);
+
+	if (dentry && ceph_dentry(dentry)) {
+		dout(" marking %p (%p) NOT complete\n", inode, dentry);
+		clear_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
+	}
+	dput(dentry);
 	return false;
 }
 
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index ec74313..9c62fe0 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -131,6 +131,8 @@ enum {
 	Opt_rbytes,
 	Opt_norbytes,
 	Opt_noasyncreaddir,
+	Opt_dcache,
+	Opt_nodcache,
 	Opt_ino32,
 };
 
@@ -152,6 +154,8 @@ static match_table_t fsopt_tokens = {
 	{Opt_rbytes, "rbytes"},
 	{Opt_norbytes, "norbytes"},
 	{Opt_noasyncreaddir, "noasyncreaddir"},
+	{Opt_dcache, "dcache"},
+	{Opt_nodcache, "nodcache"},
 	{Opt_ino32, "ino32"},
 	{-1, NULL}
 };
@@ -231,6 +235,12 @@ static int parse_fsopt_token(char *c, void *private)
 	case Opt_noasyncreaddir:
 		fsopt->flags |= CEPH_MOUNT_OPT_NOASYNCREADDIR;
 		break;
+	case Opt_dcache:
+		fsopt->flags |= CEPH_MOUNT_OPT_DCACHE;
+		break;
+	case Opt_nodcache:
+		fsopt->flags &= ~CEPH_MOUNT_OPT_DCACHE;
+		break;
 	case Opt_ino32:
 		fsopt->flags |= CEPH_MOUNT_OPT_INO32;
 		break;
@@ -377,6 +387,10 @@ static int ceph_show_options(struct seq_file *m, struct vfsmount *mnt)
 		seq_puts(m, ",norbytes");
 	if (fsopt->flags & CEPH_MOUNT_OPT_NOASYNCREADDIR)
 		seq_puts(m, ",noasyncreaddir");
+	if (fsopt->flags & CEPH_MOUNT_OPT_DCACHE)
+		seq_puts(m, ",dcache");
+	else
+		seq_puts(m, ",nodcache");
 
 	if (fsopt->wsize)
 		seq_printf(m, ",wsize=%d", fsopt->wsize);
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index edcbf37..140f99f 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -28,6 +28,7 @@
 #define CEPH_MOUNT_OPT_RBYTES          (1<<5) /* dir st_bytes = rbytes */
 #define CEPH_MOUNT_OPT_NOASYNCREADDIR  (1<<7) /* no dcache readdir */
 #define CEPH_MOUNT_OPT_INO32           (1<<8) /* 32 bit inos */
+#define CEPH_MOUNT_OPT_DCACHE          (1<<9) /* use dcache for readdir etc */
 
 #define CEPH_MOUNT_OPT_DEFAULT    (CEPH_MOUNT_OPT_RBYTES)
 
-- 
1.7.0

Re: [PATCH 2/2] ceph: enable/disable dentry complete flags via mount option

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>