Skip to content

Commit b74c79e

Browse files
author
Nick Piggin
committed
fs: provide rcu-walk aware permission i_ops
Signed-off-by: Nick Piggin <npiggin@kernel.dk>
1 parent 34286d6 commit b74c79e

60 files changed

Lines changed: 287 additions & 146 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Documentation/filesystems/Locking‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ ata *);
4747
void * (*follow_link) (struct dentry *, struct nameidata *);
4848
void (*put_link) (struct dentry *, struct nameidata *, void *);
4949
void (*truncate) (struct inode *);
50-
int (*permission) (struct inode *, int, struct nameidata *);
51-
int (*check_acl)(struct inode *, int);
50+
int (*permission) (struct inode *, int, unsigned int);
51+
int (*check_acl)(struct inode *, int, unsigned int);
5252
int (*setattr) (struct dentry *, struct iattr *);
5353
int (*getattr) (struct vfsmount *, struct dentry *, struct kstat *);
5454
int (*setxattr) (struct dentry *, const char *,const void *,size_t,int);
@@ -76,7 +76,7 @@ follow_link: no
7676
put_link: no
7777
truncate: yes (see below)
7878
setattr: yes
79-
permission: no
79+
permission: no (may not block if called in rcu-walk mode)
8080
check_acl: no
8181
getattr: no
8282
setxattr: yes

‎Documentation/filesystems/path-lookup.txt‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,11 +316,9 @@ The detailed design for rcu-walk is like this:
316316

317317
The cases where rcu-walk cannot continue are:
318318
* NULL dentry (ie. any uncached path element)
319-
* parent with d_inode->i_op->permission or ACLs
320319
* Following links
321320

322-
In future patches, permission checks become rcu-walk aware. It may be possible
323-
eventually to make following links rcu-walk aware.
321+
It may be possible eventually to make following links rcu-walk aware.
324322

325323
Uncached path elements will always require dropping to ref-walk mode, at the
326324
very least because i_mutex needs to be grabbed, and objects allocated.
@@ -336,9 +334,49 @@ or stored into. The result is massive improvements in performance and
336334
scalability of path resolution.
337335

338336

337+
Interesting statistics
338+
======================
339+
340+
The following table gives rcu lookup statistics for a few simple workloads
341+
(2s12c24t Westmere, debian non-graphical system). Ungraceful are attempts to
342+
drop rcu that fail due to d_seq failure and requiring the entire path lookup
343+
again. Other cases are successful rcu-drops that are required before the final
344+
element, nodentry for missing dentry, revalidate for filesystem revalidate
345+
routine requiring rcu drop, permission for permission check requiring drop,
346+
and link for symlink traversal requiring drop.
347+
348+
rcu-lookups restart nodentry link revalidate permission
349+
bootup 47121 0 4624 1010 10283 7852
350+
dbench 25386793 0 6778659(26.7%) 55 549 1156
351+
kbuild 2696672 10 64442(2.3%) 108764(4.0%) 1 1590
352+
git diff 39605 0 28 2 0 106
353+
vfstest 24185492 4945 708725(2.9%) 1076136(4.4%) 0 2651
354+
355+
What this shows is that failed rcu-walk lookups, ie. ones that are restarted
356+
entirely with ref-walk, are quite rare. Even the "vfstest" case which
357+
specifically has concurrent renames/mkdir/rmdir/ creat/unlink/etc to excercise
358+
such races is not showing a huge amount of restarts.
359+
360+
Dropping from rcu-walk to ref-walk mean that we have encountered a dentry where
361+
the reference count needs to be taken for some reason. This is either because
362+
we have reached the target of the path walk, or because we have encountered a
363+
condition that can't be resolved in rcu-walk mode. Ideally, we drop rcu-walk
364+
only when we have reached the target dentry, so the other statistics show where
365+
this does not happen.
366+
367+
Note that a graceful drop from rcu-walk mode due to something such as the
368+
dentry not existing (which can be common) is not necessarily a failure of
369+
rcu-walk scheme, because some elements of the path may have been walked in
370+
rcu-walk mode. The further we get from common path elements (such as cwd or
371+
root), the less contended the dentry is likely to be. The closer we are to
372+
common path elements, the more likely they will exist in dentry cache.
373+
374+
339375
Papers and other documentation on dcache locking
340376
================================================
341377

342378
1. Scaling dcache with RCU (http://linuxjournal.com/article.php?sid=7124).
343379

344380
2. http://lse.sourceforge.net/locking/dcache/dcache.html
381+
382+

‎Documentation/filesystems/porting‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,4 +379,9 @@ where possible.
379379
the filesystem provides it), which requires dropping out of rcu-walk mode. This
380380
may now be called in rcu-walk mode (nd->flags & LOOKUP_RCU). -ECHILD should be
381381
returned if the filesystem cannot handle rcu-walk. See
382+
Documentation/filesystems/vfs.txt for more details.
383+
384+
permission and check_acl are inode permission checks that are called
385+
on many or all directory inodes on the way down a path walk (to check for
386+
exec permission). These must now be rcu-walk aware (flags & IPERM_RCU). See
382387
Documentation/filesystems/vfs.txt for more details.

‎Documentation/filesystems/vfs.txt‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,8 @@ struct inode_operations {
325325
void * (*follow_link) (struct dentry *, struct nameidata *);
326326
void (*put_link) (struct dentry *, struct nameidata *, void *);
327327
void (*truncate) (struct inode *);
328-
int (*permission) (struct inode *, int, struct nameidata *);
328+
int (*permission) (struct inode *, int, unsigned int);
329+
int (*check_acl)(struct inode *, int, unsigned int);
329330
int (*setattr) (struct dentry *, struct iattr *);
330331
int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
331332
int (*setxattr) (struct dentry *, const char *,const void *,size_t,int);
@@ -414,6 +415,13 @@ otherwise noted.
414415
permission: called by the VFS to check for access rights on a POSIX-like
415416
filesystem.
416417

418+
May be called in rcu-walk mode (flags & IPERM_RCU). If in rcu-walk
419+
mode, the filesystem must check the permission without blocking or
420+
storing to the inode.
421+
422+
If a situation is encountered that rcu-walk cannot handle, return
423+
-ECHILD and it will be called again in ref-walk mode.
424+
417425
setattr: called by the VFS to set attributes for a file. This method
418426
is called by chmod(2) and related system calls.
419427

‎drivers/staging/smbfs/file.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,14 @@ smb_file_release(struct inode *inode, struct file * file)
407407
* privileges, so we need our own check for this.
408408
*/
409409
static int
410-
smb_file_permission(struct inode *inode, int mask)
410+
smb_file_permission(struct inode *inode, int mask, unsigned int flags)
411411
{
412412
int mode = inode->i_mode;
413413
int error = 0;
414414

415+
if (flags & IPERM_FLAG_RCU)
416+
return -ECHILD;
417+
415418
VERBOSE("mode=%x, mask=%x\n", mode, mask);
416419

417420
/* Look at user permissions */

‎fs/9p/acl.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,14 @@ static struct posix_acl *v9fs_get_cached_acl(struct inode *inode, int type)
9191
return acl;
9292
}
9393

94-
int v9fs_check_acl(struct inode *inode, int mask)
94+
int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags)
9595
{
9696
struct posix_acl *acl;
9797
struct v9fs_session_info *v9ses;
9898

99+
if (flags & IPERM_FLAG_RCU)
100+
return -ECHILD;
101+
99102
v9ses = v9fs_inode2v9ses(inode);
100103
if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
101104
/*

‎fs/9p/acl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
#ifdef CONFIG_9P_FS_POSIX_ACL
1818
extern int v9fs_get_acl(struct inode *, struct p9_fid *);
19-
extern int v9fs_check_acl(struct inode *inode, int mask);
19+
extern int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags);
2020
extern int v9fs_acl_chmod(struct dentry *);
2121
extern int v9fs_set_create_acl(struct dentry *,
2222
struct posix_acl *, struct posix_acl *);

‎fs/afs/internal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ extern void afs_clear_permits(struct afs_vnode *);
624624
extern void afs_cache_permit(struct afs_vnode *, struct key *, long);
625625
extern void afs_zap_permits(struct rcu_head *);
626626
extern struct key *afs_request_key(struct afs_cell *);
627-
extern int afs_permission(struct inode *, int);
627+
extern int afs_permission(struct inode *, int, unsigned int);
628628

629629
/*
630630
* server.c

‎fs/afs/security.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,13 +285,16 @@ static int afs_check_permit(struct afs_vnode *vnode, struct key *key,
285285
* - AFS ACLs are attached to directories only, and a file is controlled by its
286286
* parent directory's ACL
287287
*/
288-
int afs_permission(struct inode *inode, int mask)
288+
int afs_permission(struct inode *inode, int mask, unsigned int flags)
289289
{
290290
struct afs_vnode *vnode = AFS_FS_I(inode);
291291
afs_access_t uninitialized_var(access);
292292
struct key *key;
293293
int ret;
294294

295+
if (flags & IPERM_FLAG_RCU)
296+
return -ECHILD;
297+
295298
_enter("{{%x:%u},%lx},%x,",
296299
vnode->fid.vid, vnode->fid.vnode, vnode->flags, mask);
297300

@@ -347,7 +350,7 @@ int afs_permission(struct inode *inode, int mask)
347350
}
348351

349352
key_put(key);
350-
ret = generic_permission(inode, mask, NULL);
353+
ret = generic_permission(inode, mask, flags, NULL);
351354
_leave(" = %d", ret);
352355
return ret;
353356

‎fs/bad_inode.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,11 @@ static int bad_inode_readlink(struct dentry *dentry, char __user *buffer,
229229
return -EIO;
230230
}
231231

232-
static int bad_inode_permission(struct inode *inode, int mask)
232+
static int bad_inode_permission(struct inode *inode, int mask, unsigned int flags)
233233
{
234+
if (flags & IPERM_FLAG_RCU)
235+
return -ECHILD;
236+
234237
return -EIO;
235238
}
236239

0 commit comments

Comments
 (0)