Commit 5be1fa8a authored by Al Viro's avatar Al Viro
Browse files

Pass parent directory inode and expected name to ->d_revalidate() 



->d_revalidate() often needs to access dentry parent and name; that has
to be done carefully, since the locking environment varies from caller
to caller.  We are not guaranteed that dentry in question will not be
moved right under us - not unless the filesystem is such that nothing
on it ever gets renamed.

It can be dealt with, but that results in boilerplate code that isn't
even needed - the callers normally have just found the dentry via dcache
lookup and want to verify that it's in the right place; they already
have the values of ->d_parent and ->d_name stable.  There is a couple
of exceptions (overlayfs and, to less extent, ecryptfs), but for the
majority of calls that song and dance is not needed at all.

It's easier to make ecryptfs and overlayfs find and pass those values if
there's a ->d_revalidate() instance to be called, rather than doing that
in the instances.

This commit only changes the calling conventions; making use of supplied
values is left to followups.

NOTE: some instances need more than just the parent - things like CIFS
may need to build an entire path from filesystem root, so they need
more precautions than the usual boilerplate.  This series doesn't
do anything to that need - these filesystems have to keep their locking
mechanisms (rename_lock loops, use of dentry_path_raw(), private rwsem
a-la v9fs).

One thing to keep in mind when using name is that name->name will normally
point into the pathname being resolved; the filename in question occupies
name->len bytes starting at name->name, and there is NUL somewhere after it,
but it the next byte might very well be '/' rather than '\0'.  Do not
ignore name->len.

Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Reviewed-by: default avatarGabriel Krisman Bertazi <gabriel@krisman.be>
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 4c43ab1b

Documentation/filesystems/locking.rst

+2 −1
Original line number Diff line number Diff line
@@ -17,7 +17,8 @@ dentry_operations 


prototypes::



	int (*d_revalidate)(struct dentry *, unsigned int);

	int (*d_revalidate)(struct inode *, const struct qstr *,

			    struct dentry *, unsigned int);

	int (*d_weak_revalidate)(struct dentry *, unsigned int);

	int (*d_hash)(const struct dentry *, struct qstr *);

	int (*d_compare)(const struct dentry *,

Documentation/filesystems/porting.rst

+16 −0
Original line number Diff line number Diff line
@@ -1141,3 +1141,19 @@ pointer are gone. 


set_blocksize() takes opened struct file instead of struct block_device now

and it *must* be opened exclusive.



---



** mandatory**



->d_revalidate() gets two extra arguments - inode of parent directory and

name our dentry is expected to have.  Both are stable (dir is pinned in

non-RCU case and will stay around during the call in RCU case, and name

is guaranteed to stay unchanging).  Your instance doesn't have to use

either, but it often helps to avoid a lot of painful boilerplate.

Note that while name->name is stable and NUL-terminated, it may (and

often will) have name->name[name->len] equal to '/' rather than '\0' -

in normal case it points into the pathname being looked up.

NOTE: if you need something like full path from the root of filesystem,

you are still on your own - this assists with simple cases, but it's not

magic.

Documentation/filesystems/vfs.rst

+2 −1
Original line number Diff line number Diff line
@@ -1251,7 +1251,8 @@ defined: 
.. code-block:: c



	struct dentry_operations {

		int (*d_revalidate)(struct dentry *, unsigned int);

		int (*d_revalidate)(struct inode *, const struct qstr *,

				    struct dentry *, unsigned int);

		int (*d_weak_revalidate)(struct dentry *, unsigned int);

		int (*d_hash)(const struct dentry *, struct qstr *);

		int (*d_compare)(const struct dentry *,

fs/9p/vfs_dentry.c

+8 −2
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ static void v9fs_dentry_release(struct dentry *dentry) 
		p9_fid_put(hlist_entry(p, struct p9_fid, dlist));

}



static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)

static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)

{

	struct p9_fid *fid;

	struct inode *inode;
@@ -99,9 +99,15 @@ static int v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags) 
	return 1;

}



static int v9fs_lookup_revalidate(struct inode *dir, const struct qstr *name,

				  struct dentry *dentry, unsigned int flags)

{

	return __v9fs_lookup_revalidate(dentry, flags);

}



const struct dentry_operations v9fs_cached_dentry_operations = {

	.d_revalidate = v9fs_lookup_revalidate,

	.d_weak_revalidate = v9fs_lookup_revalidate,

	.d_weak_revalidate = __v9fs_lookup_revalidate,

	.d_delete = v9fs_cached_dentry_delete,

	.d_release = v9fs_dentry_release,

};

fs/afs/dir.c

+4 −2
Original line number Diff line number Diff line
@@ -22,7 +22,8 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry, 
				 unsigned int flags);

static int afs_dir_open(struct inode *inode, struct file *file);

static int afs_readdir(struct file *file, struct dir_context *ctx);

static int afs_d_revalidate(struct dentry *dentry, unsigned int flags);

static int afs_d_revalidate(struct inode *dir, const struct qstr *name,

			    struct dentry *dentry, unsigned int flags);

static int afs_d_delete(const struct dentry *dentry);

static void afs_d_iput(struct dentry *dentry, struct inode *inode);

static bool afs_lookup_one_filldir(struct dir_context *ctx, const char *name, int nlen,
@@ -1093,7 +1094,8 @@ static int afs_d_revalidate_rcu(struct dentry *dentry) 
 * - NOTE! the hit can be a negative hit too, so we can't assume we have an

 *   inode

 */

static int afs_d_revalidate(struct dentry *dentry, unsigned int flags)

static int afs_d_revalidate(struct inode *parent_dir, const struct qstr *name,

			    struct dentry *dentry, unsigned int flags)

{

	struct afs_vnode *vnode, *dir;

	struct afs_fid fid;