Skip to content

Commit 30d61ef

Browse files
author
Al Viro
committed
9p: fix ->rename_sem exclusion
9p wants to be able to build a path from given dentry to fs root and keep it valid over a blocking operation. ->s_vfs_rename_mutex would be a natural candidate, but there are places where we need that and where we have no way to tell if ->s_vfs_rename_mutex is already held deeper in callchain. Moreover, it's only held for cross-directory renames; name changes within the same directory happen without it. Solution: * have d_move() done in ->rename() rather than in its caller * maintain a 9p-private rwsem (per-filesystem) * hold it exclusive over the relevant part of ->rename() * hold it shared over the places where we want the path. That almost works. FS_RENAME_DOES_D_MOVE is enough to put all d_move() and d_exchange() calls under filesystem's control. However, there's also __d_unalias(), which isn't covered by any of that. If ->lookup() hits a directory inode with preexisting dentry elsewhere (due to e.g. rename done on server behind our back), d_splice_alias() called by ->lookup() will move/rename that alias. Add a couple of optional methods, so that __d_unalias() would do if alias->d_op->d_unalias_trylock != NULL if (!alias->d_op->d_unalias_trylock(alias)) fail (resulting in -ESTALE from lookup) __d_move(...) if alias->d_op->d_unalias_unlock != NULL alias->d_unalias_unlock(alias) where it currently does __d_move(). 9p instances do down_write_trylock() and up_write() of ->rename_mutex. Signed-off-by: Al Viro <[email protected]>
1 parent 90341f2 commit 30d61ef

File tree

6 files changed

+49
-1
lines changed

6 files changed

+49
-1
lines changed

Documentation/filesystems/locking.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ prototypes::
3131
struct vfsmount *(*d_automount)(struct path *path);
3232
int (*d_manage)(const struct path *, bool);
3333
struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
34+
bool (*d_unalias_trylock)(const struct dentry *);
35+
void (*d_unalias_unlock)(const struct dentry *);
3436

3537
locking rules:
3638

@@ -50,6 +52,8 @@ d_dname: no no no no
5052
d_automount: no no yes no
5153
d_manage: no no yes (ref-walk) maybe
5254
d_real no no yes no
55+
d_unalias_trylock yes no no no
56+
d_unalias_unlock yes no no no
5357
================== =========== ======== ============== ========
5458

5559
inode_operations

Documentation/filesystems/vfs.rst

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,8 @@ defined:
12651265
struct vfsmount *(*d_automount)(struct path *);
12661266
int (*d_manage)(const struct path *, bool);
12671267
struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
1268+
bool (*d_unalias_trylock)(const struct dentry *);
1269+
void (*d_unalias_unlock)(const struct dentry *);
12681270
};
12691271
12701272
``d_revalidate``
@@ -1428,6 +1430,25 @@ defined:
14281430

14291431
For non-regular files, the 'dentry' argument is returned.
14301432

1433+
``d_unalias_trylock``
1434+
if present, will be called by d_splice_alias() before moving a
1435+
preexisting attached alias. Returning false prevents __d_move(),
1436+
making d_splice_alias() fail with -ESTALE.
1437+
1438+
Rationale: setting FS_RENAME_DOES_D_MOVE will prevent d_move()
1439+
and d_exchange() calls from the outside of filesystem methods;
1440+
however, it does not guarantee that attached dentries won't
1441+
be renamed or moved by d_splice_alias() finding a preexisting
1442+
alias for a directory inode. Normally we would not care;
1443+
however, something that wants to stabilize the entire path to
1444+
root over a blocking operation might need that. See 9p for one
1445+
(and hopefully only) example.
1446+
1447+
``d_unalias_unlock``
1448+
should be paired with ``d_unalias_trylock``; that one is called after
1449+
__d_move() call in __d_unalias().
1450+
1451+
14311452
Each dentry has a pointer to its parent dentry, as well as a hash list
14321453
of child dentries. Child dentries are basically like files in a
14331454
directory.

fs/9p/v9fs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ static inline struct v9fs_session_info *v9fs_inode2v9ses(struct inode *inode)
202202
return inode->i_sb->s_fs_info;
203203
}
204204

205-
static inline struct v9fs_session_info *v9fs_dentry2v9ses(struct dentry *dentry)
205+
static inline struct v9fs_session_info *v9fs_dentry2v9ses(const struct dentry *dentry)
206206
{
207207
return dentry->d_sb->s_fs_info;
208208
}

fs/9p/vfs_dentry.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,30 @@ static int v9fs_lookup_revalidate(struct inode *dir, const struct qstr *name,
105105
return __v9fs_lookup_revalidate(dentry, flags);
106106
}
107107

108+
static bool v9fs_dentry_unalias_trylock(const struct dentry *dentry)
109+
{
110+
struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
111+
return down_write_trylock(&v9ses->rename_sem);
112+
}
113+
114+
static void v9fs_dentry_unalias_unlock(const struct dentry *dentry)
115+
{
116+
struct v9fs_session_info *v9ses = v9fs_dentry2v9ses(dentry);
117+
up_write(&v9ses->rename_sem);
118+
}
119+
108120
const struct dentry_operations v9fs_cached_dentry_operations = {
109121
.d_revalidate = v9fs_lookup_revalidate,
110122
.d_weak_revalidate = __v9fs_lookup_revalidate,
111123
.d_delete = v9fs_cached_dentry_delete,
112124
.d_release = v9fs_dentry_release,
125+
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
126+
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
113127
};
114128

115129
const struct dentry_operations v9fs_dentry_operations = {
116130
.d_delete = always_delete_dentry,
117131
.d_release = v9fs_dentry_release,
132+
.d_unalias_trylock = v9fs_dentry_unalias_trylock,
133+
.d_unalias_unlock = v9fs_dentry_unalias_unlock,
118134
};

fs/dcache.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2967,7 +2967,12 @@ static int __d_unalias(struct dentry *dentry, struct dentry *alias)
29672967
goto out_err;
29682968
m2 = &alias->d_parent->d_inode->i_rwsem;
29692969
out_unalias:
2970+
if (alias->d_op->d_unalias_trylock &&
2971+
!alias->d_op->d_unalias_trylock(alias))
2972+
goto out_err;
29702973
__d_move(alias, dentry, false);
2974+
if (alias->d_op->d_unalias_unlock)
2975+
alias->d_op->d_unalias_unlock(alias);
29712976
ret = 0;
29722977
out_err:
29732978
if (m2)

include/linux/dcache.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ struct dentry_operations {
159159
struct vfsmount *(*d_automount)(struct path *);
160160
int (*d_manage)(const struct path *, bool);
161161
struct dentry *(*d_real)(struct dentry *, enum d_real_type type);
162+
bool (*d_unalias_trylock)(const struct dentry *);
163+
void (*d_unalias_unlock)(const struct dentry *);
162164
} ____cacheline_aligned;
163165

164166
/*

0 commit comments

Comments
 (0)