| Message ID | 20140116224253.GB20521@fieldses.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > > -- 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
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 > > > > -- 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
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 > > -- 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
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 > > > > -- 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
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))) {