* [PATCH 1/4] ceph: remove ceph_mdsc_lease_release
2016-07-05 16:27 [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Jeff Layton
@ 2016-07-05 16:27 ` Jeff Layton
2016-07-05 16:27 ` [PATCH 2/4] ceph: clear d_fsinfo pointer under d_lock Jeff Layton
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2016-07-05 16:27 UTC (permalink / raw)
To: Ilya Dryomov, zyan; +Cc: Sage Weil, ceph-devel, linux-fsdevel, Al Viro
Nothing calls it.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/ceph/mds_client.c | 41 -----------------------------------------
fs/ceph/mds_client.h | 4 ----
2 files changed, 45 deletions(-)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 0d4bb24c670a..78a3495a11be 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3292,47 +3292,6 @@ void ceph_mdsc_lease_send_msg(struct ceph_mds_session *session,
}
/*
- * Preemptively release a lease we expect to invalidate anyway.
- * Pass @inode always, @dentry is optional.
- */
-void ceph_mdsc_lease_release(struct ceph_mds_client *mdsc, struct inode *inode,
- struct dentry *dentry)
-{
- struct ceph_dentry_info *di;
- struct ceph_mds_session *session;
- u32 seq;
-
- BUG_ON(inode == NULL);
- BUG_ON(dentry == NULL);
-
- /* is dentry lease valid? */
- spin_lock(&dentry->d_lock);
- di = ceph_dentry(dentry);
- if (!di || !di->lease_session ||
- di->lease_session->s_mds < 0 ||
- di->lease_gen != di->lease_session->s_cap_gen ||
- !time_before(jiffies, di->time)) {
- dout("lease_release inode %p dentry %p -- "
- "no lease\n",
- inode, dentry);
- spin_unlock(&dentry->d_lock);
- return;
- }
-
- /* we do have a lease on this dentry; note mds and seq */
- session = ceph_get_mds_session(di->lease_session);
- seq = di->lease_seq;
- __ceph_mdsc_drop_dentry_lease(dentry);
- spin_unlock(&dentry->d_lock);
-
- dout("lease_release inode %p dentry %p to mds%d\n",
- inode, dentry, session->s_mds);
- ceph_mdsc_lease_send_msg(session, inode, dentry,
- CEPH_MDS_LEASE_RELEASE, seq);
- ceph_put_mds_session(session);
-}
-
-/*
* drop all leases (and dentry refs) in preparation for umount
*/
static void drop_leases(struct ceph_mds_client *mdsc)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 2ce8e9f9bfc9..9dd2c82379f8 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -385,10 +385,6 @@ extern void ceph_mdsc_destroy(struct ceph_fs_client *fsc);
extern void ceph_mdsc_sync(struct ceph_mds_client *mdsc);
-extern void ceph_mdsc_lease_release(struct ceph_mds_client *mdsc,
- struct inode *inode,
- struct dentry *dn);
-
extern void ceph_invalidate_dir_request(struct ceph_mds_request *req);
extern int ceph_alloc_readdir_reply_buffer(struct ceph_mds_request *req,
struct inode *dir);
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] ceph: clear d_fsinfo pointer under d_lock
2016-07-05 16:27 [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Jeff Layton
2016-07-05 16:27 ` [PATCH 1/4] ceph: remove ceph_mdsc_lease_release Jeff Layton
@ 2016-07-05 16:27 ` Jeff Layton
2016-07-05 16:27 ` [PATCH 3/4] ceph: allow dentry_lease_is_valid to work under RCU walk Jeff Layton
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2016-07-05 16:27 UTC (permalink / raw)
To: Ilya Dryomov, zyan; +Cc: Sage Weil, ceph-devel, linux-fsdevel, Al Viro
To check for a valid dentry lease, we need to get at the
ceph_dentry_info. Under rcuwalk though, we may end up with a dentry that
is on its way to destruction. Since we need to take the d_lock in
dentry_lease_is_valid already, we can just ensure that we clear the
d_fsinfo pointer out under the same lock before destroying it.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/ceph/dir.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 8ff7bcc7fc88..904dc671580f 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1286,10 +1286,14 @@ static void ceph_d_release(struct dentry *dentry)
dout("d_release %p\n", dentry);
ceph_dentry_lru_del(dentry);
+
+ spin_lock(&dentry->d_lock);
+ dentry->d_fsdata = NULL;
+ spin_unlock(&dentry->d_lock);
+
if (di->lease_session)
ceph_put_mds_session(di->lease_session);
kmem_cache_free(ceph_dentry_cachep, di);
- dentry->d_fsdata = NULL;
}
static int ceph_snapdir_d_revalidate(struct dentry *dentry,
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] ceph: allow dentry_lease_is_valid to work under RCU walk
2016-07-05 16:27 [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Jeff Layton
2016-07-05 16:27 ` [PATCH 1/4] ceph: remove ceph_mdsc_lease_release Jeff Layton
2016-07-05 16:27 ` [PATCH 2/4] ceph: clear d_fsinfo pointer under d_lock Jeff Layton
@ 2016-07-05 16:27 ` Jeff Layton
2016-07-05 16:27 ` [PATCH 4/4] ceph: handle LOOKUP_RCU in ceph_d_revalidate Jeff Layton
2016-07-06 1:55 ` [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Yan, Zheng
4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2016-07-05 16:27 UTC (permalink / raw)
To: Ilya Dryomov, zyan; +Cc: Sage Weil, ceph-devel, linux-fsdevel, Al Viro
Under rcuwalk, we need to take extra care when dereferencing d_parent.
We want to do that once and pass a pointer to dentry_lease_is_valid.
Also, we must ensure that that function can handle the case where we're
racing with d_release. Check whether "di" is NULL under the d_lock, and
just return 0 if so.
Finally, we still need to kick off a renewal job if the lease is getting
close to expiration. If that's the case, then just drop out of rcuwalk
mode since that could block.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/ceph/dir.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 904dc671580f..60dcca163ba0 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1133,7 +1133,8 @@ void ceph_invalidate_dentry_lease(struct dentry *dentry)
* Check if dentry lease is valid. If not, delete the lease. Try to
* renew if the least is more than half up.
*/
-static int dentry_lease_is_valid(struct dentry *dentry)
+static int dentry_lease_is_valid(struct dentry *dentry, unsigned int flags,
+ struct inode *dir)
{
struct ceph_dentry_info *di;
struct ceph_mds_session *s;
@@ -1141,12 +1142,11 @@ static int dentry_lease_is_valid(struct dentry *dentry)
u32 gen;
unsigned long ttl;
struct ceph_mds_session *session = NULL;
- struct inode *dir = NULL;
u32 seq = 0;
spin_lock(&dentry->d_lock);
di = ceph_dentry(dentry);
- if (di->lease_session) {
+ if (di && di->lease_session) {
s = di->lease_session;
spin_lock(&s->s_gen_ttl_lock);
gen = s->s_cap_gen;
@@ -1159,12 +1159,19 @@ static int dentry_lease_is_valid(struct dentry *dentry)
valid = 1;
if (di->lease_renew_after &&
time_after(jiffies, di->lease_renew_after)) {
- /* we should renew */
- dir = d_inode(dentry->d_parent);
- session = ceph_get_mds_session(s);
- seq = di->lease_seq;
- di->lease_renew_after = 0;
- di->lease_renew_from = jiffies;
+ /*
+ * We should renew. If we're in RCU walk mode
+ * though, we can't do that so just return
+ * -ECHILD.
+ */
+ if (flags & LOOKUP_RCU) {
+ valid = -ECHILD;
+ } else {
+ session = ceph_get_mds_session(s);
+ seq = di->lease_seq;
+ di->lease_renew_after = 0;
+ di->lease_renew_from = jiffies;
+ }
}
}
}
@@ -1224,12 +1231,16 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
} else if (d_really_is_positive(dentry) &&
ceph_snap(d_inode(dentry)) == CEPH_SNAPDIR) {
valid = 1;
- } else if (dentry_lease_is_valid(dentry) ||
- dir_lease_is_valid(dir, dentry)) {
- if (d_really_is_positive(dentry))
- valid = ceph_is_any_caps(d_inode(dentry));
- else
- valid = 1;
+ } else {
+ valid = dentry_lease_is_valid(dentry, flags, dir);
+ if (valid == -ECHILD)
+ return valid;
+ if (valid || dir_lease_is_valid(dir, dentry)) {
+ if (d_really_is_positive(dentry))
+ valid = ceph_is_any_caps(d_inode(dentry));
+ else
+ valid = 1;
+ }
}
if (!valid) {
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] ceph: handle LOOKUP_RCU in ceph_d_revalidate
2016-07-05 16:27 [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Jeff Layton
` (2 preceding siblings ...)
2016-07-05 16:27 ` [PATCH 3/4] ceph: allow dentry_lease_is_valid to work under RCU walk Jeff Layton
@ 2016-07-05 16:27 ` Jeff Layton
2016-07-06 1:55 ` [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Yan, Zheng
4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2016-07-05 16:27 UTC (permalink / raw)
To: Ilya Dryomov, zyan; +Cc: Sage Weil, ceph-devel, linux-fsdevel, Al Viro
We can now handle the snapshot cases under RCU, as well as the
non-snapshot case when we don't need to queue up a lease renewal.
Allow LOOKUP_RCU walks to proceed under those conditions.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/ceph/dir.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 60dcca163ba0..c64a0b794d49 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1214,15 +1214,19 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
struct dentry *parent;
struct inode *dir;
- if (flags & LOOKUP_RCU)
- return -ECHILD;
+ if (flags & LOOKUP_RCU) {
+ parent = ACCESS_ONCE(dentry->d_parent);
+ dir = d_inode_rcu(parent);
+ if (!dir)
+ return -ECHILD;
+ } else {
+ parent = dget_parent(dentry);
+ dir = d_inode(parent);
+ }
dout("d_revalidate %p '%pd' inode %p offset %lld\n", dentry,
dentry, d_inode(dentry), ceph_dentry(dentry)->offset);
- parent = dget_parent(dentry);
- dir = d_inode(parent);
-
/* always trust cached snapped dentries, snapdir dentry */
if (ceph_snap(dir) != CEPH_NOSNAP) {
dout("d_revalidate %p '%pd' inode %p is SNAPPED\n", dentry,
@@ -1249,6 +1253,9 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
struct ceph_mds_request *req;
int op, mask, err;
+ if (flags & LOOKUP_RCU)
+ return -ECHILD;
+
op = ceph_snap(dir) == CEPH_SNAPDIR ?
CEPH_MDS_OP_LOOKUPSNAP : CEPH_MDS_OP_LOOKUP;
req = ceph_mdsc_create_request(mdsc, op, USE_ANY_MDS);
@@ -1284,7 +1291,8 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
ceph_dir_clear_complete(dir);
}
- dput(parent);
+ if (!(flags & LOOKUP_RCU))
+ dput(parent);
return valid;
}
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU
2016-07-05 16:27 [PATCH 0/4] ceph: allow ceph_d_revalidate to handle some cases under LOOKUP_RCU Jeff Layton
` (3 preceding siblings ...)
2016-07-05 16:27 ` [PATCH 4/4] ceph: handle LOOKUP_RCU in ceph_d_revalidate Jeff Layton
@ 2016-07-06 1:55 ` Yan, Zheng
4 siblings, 0 replies; 6+ messages in thread
From: Yan, Zheng @ 2016-07-06 1:55 UTC (permalink / raw)
To: Jeff Layton; +Cc: Ilya Dryomov, Sage Weil, ceph-devel, linux-fsdevel, Al Viro
> On Jul 6, 2016, at 00:27, Jeff Layton <jlayton@redhat.com> wrote:
>
> The basic idea with this set is to allow the kernel client to allow the
> client to do more d_revalidate functionality under the rcu_read_lock.
> The idea is basically to do what we can there while not blocking, and
> then return -ECHILD for the rest.
>
> The only really questionable bit is the part where we need to take the
> d_lock in ceph_d_release. I think it should be largely uncontended, but
> it's still less than ideal. Maybe there is some way to handle that
> locklessly?
>
> Also, it would be nice to not have to drop out of rcuwalk mode in order
> to run the lease renewal. It seems like the kind of thing that would be
> ideal to queue to a workqueue, but I don't see a way to do that without
> introducing a potential deadlock.
>
> My teuthology run with it passed all but one test, which seems to have
> been an unrelated problem (though I could be wrong there).
>
> Jeff Layton (4):
> ceph: remove ceph_mdsc_lease_release
> ceph: clear d_fsinfo pointer under d_lock
> ceph: allow dentry_lease_is_valid to work under RCU walk
> ceph: handle LOOKUP_RCU in ceph_d_revalidate
>
> fs/ceph/dir.c | 67 +++++++++++++++++++++++++++++++++++-----------------
> fs/ceph/mds_client.c | 41 --------------------------------
> fs/ceph/mds_client.h | 4 ----
> 3 files changed, 45 insertions(+), 67 deletions(-)
Reviewed-by: Yan, Zheng <zyan@redhat.com>
>
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread