* [PATCH] ceph: trivial comment fix
@ 2014-01-16 22:42 J. Bruce Fields
2014-01-17 0:03 ` Sage Weil
0 siblings, 1 reply; 5+ messages in thread
From: J. Bruce Fields @ 2014-01-16 22:42 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
From: "J. Bruce Fields" <bfields@redhat.com>
"disconnected" is too easily confused with "DCACHE_DISCONNECTED". I
think "unhashed" is the more precise term here.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/ceph/caps.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Just ran across this while wondering what d_find_alias callers do....
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 3c0a4bd..697f9d7 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2350,11 +2350,11 @@ static void invalidate_aliases(struct inode *inode)
d_prune_aliases(inode);
/*
* For non-directory inode, d_find_alias() only returns
- * connected dentry. After calling d_invalidate(), the
- * dentry become disconnected.
+ * hashed dentry. After calling d_invalidate(), the
+ * dentry becomes unhashed.
*
* For directory inode, d_find_alias() can return
- * disconnected dentry. But directory inode should have
+ * unhashed dentry. But directory inode should have
* one alias at most.
*/
while ((dn = d_find_alias(inode))) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: trivial comment fix
2014-01-16 22:42 [PATCH] ceph: trivial comment fix J. Bruce Fields
@ 2014-01-17 0:03 ` Sage Weil
2014-01-17 2:01 ` J. Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Sage Weil @ 2014-01-17 0:03 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: ceph-devel
On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> "disconnected" is too easily confused with "DCACHE_DISCONNECTED". I
> think "unhashed" is the more precise term here.
Good point. Applied, thanks!
sage
>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> ---
> fs/ceph/caps.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Just ran across this while wondering what d_find_alias callers do....
>
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 3c0a4bd..697f9d7 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2350,11 +2350,11 @@ static void invalidate_aliases(struct inode *inode)
> d_prune_aliases(inode);
> /*
> * For non-directory inode, d_find_alias() only returns
> - * connected dentry. After calling d_invalidate(), the
> - * dentry become disconnected.
> + * hashed dentry. After calling d_invalidate(), the
> + * dentry becomes unhashed.
> *
> * For directory inode, d_find_alias() can return
> - * disconnected dentry. But directory inode should have
> + * unhashed dentry. But directory inode should have
> * one alias at most.
> */
> while ((dn = d_find_alias(inode))) {
> --
> 1.7.9.5
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: trivial comment fix
2014-01-17 0:03 ` Sage Weil
@ 2014-01-17 2:01 ` J. Bruce Fields
2014-01-18 4:33 ` Sage Weil
0 siblings, 1 reply; 5+ messages in thread
From: J. Bruce Fields @ 2014-01-17 2:01 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On Thu, Jan 16, 2014 at 04:03:38PM -0800, Sage Weil wrote:
> On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> >
> > "disconnected" is too easily confused with "DCACHE_DISCONNECTED". I
> > think "unhashed" is the more precise term here.
>
> Good point. Applied, thanks!
Thanks! While I'm looking, there's another d_find_alias() caller in
build_inode_path. What's that? (Do the mds protocol messages actually
use full paths?) And will this break if you get a DCACHE_DISCONNECTED
or an unhashed alias?
--b.
> sage
>
>
> >
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > fs/ceph/caps.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > Just ran across this while wondering what d_find_alias callers do....
> >
> > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > index 3c0a4bd..697f9d7 100644
> > --- a/fs/ceph/caps.c
> > +++ b/fs/ceph/caps.c
> > @@ -2350,11 +2350,11 @@ static void invalidate_aliases(struct inode *inode)
> > d_prune_aliases(inode);
> > /*
> > * For non-directory inode, d_find_alias() only returns
> > - * connected dentry. After calling d_invalidate(), the
> > - * dentry become disconnected.
> > + * hashed dentry. After calling d_invalidate(), the
> > + * dentry becomes unhashed.
> > *
> > * For directory inode, d_find_alias() can return
> > - * disconnected dentry. But directory inode should have
> > + * unhashed dentry. But directory inode should have
> > * one alias at most.
> > */
> > while ((dn = d_find_alias(inode))) {
> > --
> > 1.7.9.5
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: trivial comment fix
2014-01-17 2:01 ` J. Bruce Fields
@ 2014-01-18 4:33 ` Sage Weil
2014-01-22 18:23 ` J. Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Sage Weil @ 2014-01-18 4:33 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: ceph-devel
On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> On Thu, Jan 16, 2014 at 04:03:38PM -0800, Sage Weil wrote:
> > On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > >
> > > "disconnected" is too easily confused with "DCACHE_DISCONNECTED". I
> > > think "unhashed" is the more precise term here.
> >
> > Good point. Applied, thanks!
>
> Thanks! While I'm looking, there's another d_find_alias() caller in
> build_inode_path. What's that? (Do the mds protocol messages actually
> use full paths?) And will this break if you get a DCACHE_DISCONNECTED
> or an unhashed alias?
They build a path relative to the first non-snapped parent, which in most
cases is either no path at all (just an ino) or a single path segment.
The exception is if you are inside a snapshotted directory (e.g.
a/b/.snap/mysnap/c/d), in which case it can be deeper. In any case, I
think the only problem is if you have a disconnected dentry from an old
nfs filename for an inode within a snapshot. There may be some issues
there (nfs reexport + snaps isn't currently part of the test suite), but
in general I don't think there is any issue with disconnected dentries.
sage
>
> --b.
>
> > sage
> >
> >
> > >
> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > ---
> > > fs/ceph/caps.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > Just ran across this while wondering what d_find_alias callers do....
> > >
> > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > > index 3c0a4bd..697f9d7 100644
> > > --- a/fs/ceph/caps.c
> > > +++ b/fs/ceph/caps.c
> > > @@ -2350,11 +2350,11 @@ static void invalidate_aliases(struct inode *inode)
> > > d_prune_aliases(inode);
> > > /*
> > > * For non-directory inode, d_find_alias() only returns
> > > - * connected dentry. After calling d_invalidate(), the
> > > - * dentry become disconnected.
> > > + * hashed dentry. After calling d_invalidate(), the
> > > + * dentry becomes unhashed.
> > > *
> > > * For directory inode, d_find_alias() can return
> > > - * disconnected dentry. But directory inode should have
> > > + * unhashed dentry. But directory inode should have
> > > * one alias at most.
> > > */
> > > while ((dn = d_find_alias(inode))) {
> > > --
> > > 1.7.9.5
> > >
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: trivial comment fix
2014-01-18 4:33 ` Sage Weil
@ 2014-01-22 18:23 ` J. Bruce Fields
0 siblings, 0 replies; 5+ messages in thread
From: J. Bruce Fields @ 2014-01-22 18:23 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On Fri, Jan 17, 2014 at 08:33:48PM -0800, Sage Weil wrote:
> On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> > On Thu, Jan 16, 2014 at 04:03:38PM -0800, Sage Weil wrote:
> > > On Thu, 16 Jan 2014, J. Bruce Fields wrote:
> > > > From: "J. Bruce Fields" <bfields@redhat.com>
> > > >
> > > > "disconnected" is too easily confused with "DCACHE_DISCONNECTED". I
> > > > think "unhashed" is the more precise term here.
> > >
> > > Good point. Applied, thanks!
> >
> > Thanks! While I'm looking, there's another d_find_alias() caller in
> > build_inode_path. What's that? (Do the mds protocol messages actually
> > use full paths?) And will this break if you get a DCACHE_DISCONNECTED
> > or an unhashed alias?
>
> They build a path relative to the first non-snapped parent, which in most
> cases is either no path at all (just an ino) or a single path segment.
> The exception is if you are inside a snapshotted directory (e.g.
> a/b/.snap/mysnap/c/d), in which case it can be deeper. In any case, I
> think the only problem is if you have a disconnected dentry from an old
> nfs filename for an inode within a snapshot.
Right, so I guess there may be a problem you get a disconnected dentry
that's not yet connected all the way back up to the main dcache, in
which case the !IS_ROOT tests in the loops in ceph_mdsc_build_path() may
fail too soon.
--b.
> There may be some issues
> there (nfs reexport + snaps isn't currently part of the test suite), but
> in general I don't think there is any issue with disconnected dentries.
>
> sage
>
>
> >
> > --b.
> >
> > > sage
> > >
> > >
> > > >
> > > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > > > ---
> > > > fs/ceph/caps.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > Just ran across this while wondering what d_find_alias callers do....
> > > >
> > > > diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> > > > index 3c0a4bd..697f9d7 100644
> > > > --- a/fs/ceph/caps.c
> > > > +++ b/fs/ceph/caps.c
> > > > @@ -2350,11 +2350,11 @@ static void invalidate_aliases(struct inode *inode)
> > > > d_prune_aliases(inode);
> > > > /*
> > > > * For non-directory inode, d_find_alias() only returns
> > > > - * connected dentry. After calling d_invalidate(), the
> > > > - * dentry become disconnected.
> > > > + * hashed dentry. After calling d_invalidate(), the
> > > > + * dentry becomes unhashed.
> > > > *
> > > > * For directory inode, d_find_alias() can return
> > > > - * disconnected dentry. But directory inode should have
> > > > + * unhashed dentry. But directory inode should have
> > > > * one alias at most.
> > > > */
> > > > while ((dn = d_find_alias(inode))) {
> > > > --
> > > > 1.7.9.5
> > > >
> > > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-22 18:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-16 22:42 [PATCH] ceph: trivial comment fix J. Bruce Fields
2014-01-17 0:03 ` Sage Weil
2014-01-17 2:01 ` J. Bruce Fields
2014-01-18 4:33 ` Sage Weil
2014-01-22 18:23 ` J. Bruce Fields
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.