* [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
* Re: [PATCH 1/3] ceph: take inode lock when finding an inode alias
2011-12-29 2:05 ` [PATCH 1/3] ceph: take inode lock when finding an inode alias Sage Weil
@ 2011-12-29 9:10 ` Christoph Hellwig
0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2011-12-29 9:10 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel, Alex Elder
On Wed, Dec 28, 2011 at 06:05:13PM -0800, Sage Weil wrote:
> +/* 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" */
I would be much happier about just exporting d_find_any_alias.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] ceph: take inode lock when finding an inode alias
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:10 ` 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: 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.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-29 9:10 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 1/3] ceph: take inode lock when finding an inode alias Sage Weil
2011-12-29 9:10 ` 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.