* [PATCH 1/3] ceph: take inode lock when finding an inode alias
@ 2011-12-22 20:13 Sage Weil
2011-12-22 20:13 ` [PATCH 2/3] ceph: take a reference to the dentry in d_find_any_alias() Sage Weil
2011-12-22 20:13 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil
0 siblings, 2 replies; 6+ messages in thread
From: Sage Weil @ 2011-12-22 20:13 UTC (permalink / raw)
To: ceph-devel; +Cc: Alex Elder
From: Alex Elder <elder@dreamhost.com>
In the ceph_dir_*_complete() functions, a call to
__d_find_any_alias() is used to get a dentry for a inode.
Previously this was done under the inode's i_lock, but
recently this change converted things to use the ceph
inode's i_ceph_lock instead:
be655596 ceph: use i_ceph_lock instead of i_lock
Finding an inode alias operates (only) on the Linux inode, so
we really do need to take the Linux lock for this. Since i_lock
is ordered inside i_ceph_lock, we can safely do this for all
these ceph cases.
For now, just copy the d_find_any_alias() function from
"fs/dcache.c" and use that instead.
Signed-off-by: Alex Elder <elder@dreamhost.com>
---
fs/ceph/dir.c | 18 +++++++++++++++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 3eeb976..e58b0d1 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1104,9 +1104,21 @@ static struct dentry * __d_find_any_alias(struct inode *inode)
return alias;
}
+/* The following code copied from "fs/dcache.c" */
+static struct dentry * d_find_any_alias(struct inode *inode)
+{
+ struct dentry *de;
+
+ spin_lock(&inode->i_lock);
+ de = __d_find_any_alias(inode);
+ spin_unlock(&inode->i_lock);
+ return de;
+}
+/* End of code copied from "fs/dcache.c" */
+
void ceph_dir_set_complete(struct inode *inode)
{
- struct dentry *dentry = __d_find_any_alias(inode);
+ struct dentry *dentry = d_find_any_alias(inode);
if (dentry && ceph_dentry(dentry)) {
dout(" marking %p (%p) complete\n", inode, dentry);
@@ -1116,7 +1128,7 @@ void ceph_dir_set_complete(struct inode *inode)
void ceph_dir_clear_complete(struct inode *inode)
{
- struct dentry *dentry = __d_find_any_alias(inode);
+ struct dentry *dentry = d_find_any_alias(inode);
if (dentry && ceph_dentry(dentry)) {
dout(" marking %p (%p) NOT complete\n", inode, dentry);
@@ -1126,7 +1138,7 @@ void ceph_dir_clear_complete(struct inode *inode)
bool ceph_dir_test_complete(struct inode *inode)
{
- struct dentry *dentry = __d_find_any_alias(inode);
+ struct dentry *dentry = d_find_any_alias(inode);
if (dentry && ceph_dentry(dentry))
return test_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ceph: take a reference to the dentry in d_find_any_alias() 2011-12-22 20:13 [PATCH 1/3] ceph: take inode lock when finding an inode alias Sage Weil @ 2011-12-22 20:13 ` Sage Weil 2011-12-22 20:13 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil 1 sibling, 0 replies; 6+ messages in thread From: Sage Weil @ 2011-12-22 20:13 UTC (permalink / raw) To: ceph-devel; +Cc: Alex Elder From: Alex Elder <elder@dreamhost.com> The ceph code duplicates __d_find_any_alias(), but it currently does not take a reference to the returned dentry as it should. Replace the ceph implementation with an exact copy of what's found in "fs/dcache.c", and update the callers so they drop their reference when they're done with it. Unfortunately this requires the wholesale copy of the functions that implement __dget(). It would be much nicer to just export d_find_any_alias() from "fs/dcache.c" instead. Signed-off-by: Alex Elder <elder@dreamhost.com> --- fs/ceph/dir.c | 31 +++++++++++++++++++++++++------ 1 files changed, 25 insertions(+), 6 deletions(-) diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index e58b0d1..caddb7d 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -1091,9 +1091,20 @@ static int ceph_snapdir_d_revalidate(struct dentry *dentry, return 1; } -/* - * Set/clear/test dir complete flag on the dir's dentry. - */ +/* The following code copied from "fs/dcache.c" */ +/* This must be called with d_lock held */ +static inline void __dget_dlock(struct dentry *dentry) +{ + dentry->d_count++; +} + +static inline void __dget(struct dentry *dentry) +{ + spin_lock(&dentry->d_lock); + __dget_dlock(dentry); + spin_unlock(&dentry->d_lock); +} + static struct dentry * __d_find_any_alias(struct inode *inode) { struct dentry *alias; @@ -1101,10 +1112,10 @@ static struct dentry * __d_find_any_alias(struct inode *inode) if (list_empty(&inode->i_dentry)) return NULL; alias = list_first_entry(&inode->i_dentry, struct dentry, d_alias); + __dget(alias); return alias; } -/* The following code copied from "fs/dcache.c" */ static struct dentry * d_find_any_alias(struct inode *inode) { struct dentry *de; @@ -1116,6 +1127,9 @@ static struct dentry * d_find_any_alias(struct inode *inode) } /* End of code copied from "fs/dcache.c" */ +/* + * Set/clear/test dir complete flag on the dir's dentry. + */ void ceph_dir_set_complete(struct inode *inode) { struct dentry *dentry = d_find_any_alias(inode); @@ -1124,6 +1138,7 @@ void ceph_dir_set_complete(struct inode *inode) 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) @@ -1134,15 +1149,19 @@ void ceph_dir_clear_complete(struct inode *inode) dout(" marking %p (%p) NOT complete\n", inode, dentry); clear_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); } + dput(dentry); } bool ceph_dir_test_complete(struct inode *inode) { struct dentry *dentry = d_find_any_alias(inode); + bool ret = false; if (dentry && ceph_dentry(dentry)) - return test_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); - return false; + ret = test_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); + dput(dentry); + + return ret; } /* -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option 2011-12-22 20:13 [PATCH 1/3] ceph: take inode lock when finding an inode alias Sage Weil 2011-12-22 20:13 ` [PATCH 2/3] ceph: take a reference to the dentry in d_find_any_alias() Sage Weil @ 2011-12-22 20:13 ` Sage Weil 2011-12-22 21:01 ` Alex Elder 1 sibling, 1 reply; 6+ messages in thread From: Sage Weil @ 2011-12-22 20:13 UTC (permalink / raw) To: ceph-devel; +Cc: Sage Weil Enable/disable use of the dentry dir 'complete' flag via a mount option. Signed-off-by: Sage Weil <sage@newdream.net> --- fs/ceph/dir.c | 3 ++- fs/ceph/super.c | 14 ++++++++++++++ fs/ceph/super.h | 1 + 3 files changed, 17 insertions(+), 1 deletions(-) diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index caddb7d..35db035 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -1134,7 +1134,8 @@ void ceph_dir_set_complete(struct inode *inode) { struct dentry *dentry = d_find_any_alias(inode); - if (dentry && ceph_dentry(dentry)) { + if (dentry && ceph_dentry(dentry) && + ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), USEDCACHE)) { dout(" marking %p (%p) complete\n", inode, dentry); set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); } diff --git a/fs/ceph/super.c b/fs/ceph/super.c index b48f15f..3f4e5ad 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -131,6 +131,8 @@ enum { Opt_rbytes, Opt_norbytes, Opt_noasyncreaddir, + Opt_usedcache, + Opt_nousedcache, Opt_ino32, }; @@ -152,6 +154,8 @@ static match_table_t fsopt_tokens = { {Opt_rbytes, "rbytes"}, {Opt_norbytes, "norbytes"}, {Opt_noasyncreaddir, "noasyncreaddir"}, + {Opt_usedcache, "usedcache"}, + {Opt_nousedcache, "nousedcache"}, {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_usedcache: + fsopt->flags |= CEPH_MOUNT_OPT_USEDCACHE; + break; + case Opt_nousedcache: + fsopt->flags &= ~CEPH_MOUNT_OPT_USEDCACHE; + 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_USEDCACHE) + seq_puts(m, ",usedcache"); + else + seq_puts(m, ",nousedcache"); if (fsopt->wsize) seq_printf(m, ",wsize=%d", fsopt->wsize); diff --git a/fs/ceph/super.h b/fs/ceph/super.h index edcbf37..9270d9d 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_USEDCACHE (1<<9) #define CEPH_MOUNT_OPT_DEFAULT (CEPH_MOUNT_OPT_RBYTES) -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option 2011-12-22 20:13 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil @ 2011-12-22 21:01 ` Alex Elder 0 siblings, 0 replies; 6+ messages in thread From: Alex Elder @ 2011-12-22 21:01 UTC (permalink / raw) To: Sage Weil; +Cc: ceph-devel On Thu, 2011-12-22 at 12:13 -0800, Sage Weil wrote: > Enable/disable use of the dentry dir 'complete' flag via a mount option. > > Signed-off-by: Sage Weil <sage@newdream.net> > --- > fs/ceph/dir.c | 3 ++- > fs/ceph/super.c | 14 ++++++++++++++ > fs/ceph/super.h | 1 + > 3 files changed, 17 insertions(+), 1 deletions(-) > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index caddb7d..35db035 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -1134,7 +1134,8 @@ void ceph_dir_set_complete(struct inode *inode) > { > struct dentry *dentry = d_find_any_alias(inode); > > - if (dentry && ceph_dentry(dentry)) { > + if (dentry && ceph_dentry(dentry) && > + ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), USEDCACHE)) { > dout(" marking %p (%p) complete\n", inode, dentry); > set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); > } You should clear the CEPH_D_COMPLETE bit conditionally in ceph_dir_clear_complete() also. And for ceph_dir_test_complete() I think that whole function should be made conditional on the mount option too, although I honestly don't know whether it should return true or false in the "nousedcache" case. I realize making the test and clear ones conditional is not technically necessary, but I think it's good to handle it all symmetrically. Aside from that though I think the change looks fine. -Alex > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > index b48f15f..3f4e5ad 100644 > --- a/fs/ceph/super.c > +++ b/fs/ceph/super.c > @@ -131,6 +131,8 @@ enum { > Opt_rbytes, > Opt_norbytes, > Opt_noasyncreaddir, > + Opt_usedcache, > + Opt_nousedcache, > Opt_ino32, > }; > > @@ -152,6 +154,8 @@ static match_table_t fsopt_tokens = { > {Opt_rbytes, "rbytes"}, > {Opt_norbytes, "norbytes"}, > {Opt_noasyncreaddir, "noasyncreaddir"}, > + {Opt_usedcache, "usedcache"}, > + {Opt_nousedcache, "nousedcache"}, > {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_usedcache: > + fsopt->flags |= CEPH_MOUNT_OPT_USEDCACHE; > + break; > + case Opt_nousedcache: > + fsopt->flags &= ~CEPH_MOUNT_OPT_USEDCACHE; > + 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_USEDCACHE) > + seq_puts(m, ",usedcache"); > + else > + seq_puts(m, ",nousedcache"); > > if (fsopt->wsize) > seq_printf(m, ",wsize=%d", fsopt->wsize); > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index edcbf37..9270d9d 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_USEDCACHE (1<<9) > > #define CEPH_MOUNT_OPT_DEFAULT (CEPH_MOUNT_OPT_RBYTES) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/3] Ceph fixes for 3.2 final @ 2011-12-29 2:05 Sage Weil 2011-12-29 2:05 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil 0 siblings, 1 reply; 6+ messages in thread From: Sage Weil @ 2011-12-29 2:05 UTC (permalink / raw) To: ceph-devel; +Cc: Sage Weil This is a final set of fixes for 3.2. The main issue is with the d_find_any_alias() code, which used to rely on the caller holding i_lock (this is no longer the case). The last patch adds a mount option to enable/disable use of the dcache for negative lookups and readdir on directories the client has completely cached; there are some bugs lurking in that code so it is disabled for now. I'll be sending this to Linus tomorrow, unless anyone see any problems! sage Alex Elder (2): ceph: take inode lock when finding an inode alias ceph: take a reference to the dentry in d_find_any_alias() Sage Weil (1): ceph: enable/disable dentry complete flags via mount option fs/ceph/dir.c | 50 +++++++++++++++++++++++++++++++++++++++++--------- fs/ceph/super.c | 14 ++++++++++++++ fs/ceph/super.h | 1 + 3 files changed, 56 insertions(+), 9 deletions(-) -- 1.7.2.5 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option 2011-12-29 2:05 [PATCH 0/3] Ceph fixes for 3.2 final Sage Weil @ 2011-12-29 2:05 ` Sage Weil 2011-12-29 9:11 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: Sage Weil @ 2011-12-29 2:05 UTC (permalink / raw) To: ceph-devel; +Cc: Sage Weil Enable/disable use of the dentry dir 'complete' flag via a mount option. Signed-off-by: Sage Weil <sage@newdream.net> --- fs/ceph/dir.c | 3 ++- fs/ceph/super.c | 14 ++++++++++++++ fs/ceph/super.h | 1 + 3 files changed, 17 insertions(+), 1 deletions(-) diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index caddb7d..35db035 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -1134,7 +1134,8 @@ void ceph_dir_set_complete(struct inode *inode) { struct dentry *dentry = d_find_any_alias(inode); - if (dentry && ceph_dentry(dentry)) { + if (dentry && ceph_dentry(dentry) && + ceph_test_mount_opt(ceph_sb_to_client(dentry->d_sb), USEDCACHE)) { dout(" marking %p (%p) complete\n", inode, dentry); set_bit(CEPH_D_COMPLETE, &ceph_dentry(dentry)->flags); } diff --git a/fs/ceph/super.c b/fs/ceph/super.c index b48f15f..3f4e5ad 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -131,6 +131,8 @@ enum { Opt_rbytes, Opt_norbytes, Opt_noasyncreaddir, + Opt_usedcache, + Opt_nousedcache, Opt_ino32, }; @@ -152,6 +154,8 @@ static match_table_t fsopt_tokens = { {Opt_rbytes, "rbytes"}, {Opt_norbytes, "norbytes"}, {Opt_noasyncreaddir, "noasyncreaddir"}, + {Opt_usedcache, "usedcache"}, + {Opt_nousedcache, "nousedcache"}, {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_usedcache: + fsopt->flags |= CEPH_MOUNT_OPT_USEDCACHE; + break; + case Opt_nousedcache: + fsopt->flags &= ~CEPH_MOUNT_OPT_USEDCACHE; + 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_USEDCACHE) + seq_puts(m, ",usedcache"); + else + seq_puts(m, ",nousedcache"); if (fsopt->wsize) seq_printf(m, ",wsize=%d", fsopt->wsize); diff --git a/fs/ceph/super.h b/fs/ceph/super.h index edcbf37..9270d9d 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_USEDCACHE (1<<9) #define CEPH_MOUNT_OPT_DEFAULT (CEPH_MOUNT_OPT_RBYTES) -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option 2011-12-29 2:05 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil @ 2011-12-29 9:11 ` Christoph Hellwig 0 siblings, 0 replies; 6+ messages in thread From: Christoph Hellwig @ 2011-12-29 9:11 UTC (permalink / raw) To: Sage Weil; +Cc: ceph-devel On Wed, Dec 28, 2011 at 06:05:15PM -0800, Sage Weil wrote: > Enable/disable use of the dentry dir 'complete' flag via a mount option. Please add documentation when a user would specify this option. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-29 9:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-12-22 20:13 [PATCH 1/3] ceph: take inode lock when finding an inode alias Sage Weil 2011-12-22 20:13 ` [PATCH 2/3] ceph: take a reference to the dentry in d_find_any_alias() Sage Weil 2011-12-22 20:13 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil 2011-12-22 21:01 ` Alex Elder 2011-12-29 2:05 [PATCH 0/3] Ceph fixes for 3.2 final Sage Weil 2011-12-29 2:05 ` [PATCH 3/3] ceph: enable/disable dentry complete flags via mount option Sage Weil 2011-12-29 9:11 ` Christoph Hellwig
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.