07 Jan, 2012

1 commit


04 Jan, 2012

10 commits


08 Nov, 2011

1 commit

  • Mountpoint crossing is similar to following procfs symlinks - we do
    not get ->d_revalidate() called for dentry we have arrived at, with
    unpleasant consequences for NFS4.

    Simple way to reproduce the problem in mainline:

    cat >/tmp/a.c <
    #include
    #include
    main()
    {
    struct flock fl = {.l_type = F_RDLCK, .l_whence = SEEK_SET, .l_len = 1};
    if (fcntl(0, F_SETLK, &fl))
    perror("setlk");
    }
    EOF
    cc /tmp/a.c -o /tmp/test

    then on nfs4:

    mount --bind file1 file2
    /tmp/test < file1 # ok
    /tmp/test < file2 # spews "setlk: No locks available"...

    What happens is the missing call of ->d_revalidate() after mountpoint
    crossing and that's where NFS4 would issue OPEN request to server.

    The fix is simple - treat mountpoint crossing the same way we deal with
    following procfs-style symlinks. I.e. set LOOKUP_JUMPED...

    Cc: stable@kernel.org
    Signed-off-by: Al Viro
    Signed-off-by: Linus Torvalds

    Al Viro
     

02 Nov, 2011

1 commit

  • Since the commit below which added O_PATH support to the *at() calls, the
    error return for readlink/readlinkat for the empty pathname has switched
    from ENOENT to EINVAL:

    commit 65cfc6722361570bfe255698d9cd4dccaf47570d
    Author: Al Viro
    Date: Sun Mar 13 15:56:26 2011 -0400

    readlinkat(), fchownat() and fstatat() with empty relative pathnames

    This is both unexpected for userspace and makes readlink/readlinkat
    inconsistant with all other interfaces; and inconsistant with our stated
    return for these pathnames.

    As the readlinkat call does not have a flags parameter we cannot use the
    AT_EMPTY_PATH approach used in the other calls. Therefore expose whether
    the original path is infact entry via a new user_path_at_empty() path
    lookup function. Use this to determine whether to default to EINVAL or
    ENOENT for failures.

    Addresses http://bugs.launchpad.net/bugs/817187

    [akpm@linux-foundation.org: remove unused getname_flags()]
    Signed-off-by: Andy Whitcroft
    Cc: Christoph Hellwig
    Cc: Al Viro
    Cc:
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Christoph Hellwig

    Andy Whitcroft
     

28 Oct, 2011

4 commits


27 Sep, 2011

2 commits

  • That flag no longer makes sense, since we don't look up automount points
    as eagerly any more. Additionally, it turns out that the NO_AUTOMOUNT
    handling was buggy to begin with: it would avoid automounting even for
    cases where we really *needed* to do the automount handling, and could
    return ENOENT for autofs entries that hadn't been instantiated yet.

    With our new non-eager automount semantics, one discussion has been
    about adding a AT_AUTOMOUNT flag to vfs_fstatat (and thus the
    newfstatat() and fstatat64() system calls), but it's probably not worth
    it: you can always force at least directory automounting by simply
    adding the final '/' to the filename, which works for *all* of the stat
    family system calls, old and new.

    So AT_NO_AUTOMOUNT (and thus LOOKUP_NO_AUTOMOUNT) really were just a
    result of our bad default behavior.

    Acked-by: Ian Kent
    Acked-by: Trond Myklebust
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • Since we've now turned around and made LOOKUP_FOLLOW *not* force an
    automount, we want to add the ability to force an automount event on
    lookup even if we don't happen to have one of the other flags that force
    it implicitly (LOOKUP_OPEN, LOOKUP_DIRECTORY, LOOKUP_PARENT..)

    Most cases will never want to use this, since you'd normally want to
    delay automounting as long as possible, which usually implies
    LOOKUP_OPEN (when we open a file or directory, we really cannot avoid
    the automount any more).

    But Trond argued sufficiently forcefully that at a minimum bind mounting
    a file and quotactl will want to force the automount lookup. Some other
    cases (like nfs_follow_remote_path()) could use it too, although
    LOOKUP_DIRECTORY would work there as well.

    This commit just adds the flag and logic, no users yet, though. It also
    doesn't actually touch the LOOKUP_NO_AUTOMOUNT flag that is related, and
    was made irrelevant by the same change that made us not follow on
    LOOKUP_FOLLOW.

    Cc: Trond Myklebust
    Cc: Ian Kent
    Cc: Jeff Layton
    Cc: Miklos Szeredi
    Cc: David Howells
    Cc: Al Viro
    Cc: Greg KH
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

15 Sep, 2011

1 commit

  • We used to get the victim pinned by dentry_unhash() prior to commit
    64252c75a219 ("vfs: remove dget() from dentry_unhash()") and ->rmdir()
    and ->rename() instances relied on that; most of them don't care, but
    ones that used d_delete() themselves do. As the result, we are getting
    rmdir() oopses on NFS now.

    Just grab the reference before locking the victim and drop it explicitly
    after unlocking, same as vfs_rename_other() does.

    Signed-off-by: Al Viro
    Tested-by: Simon Kirby
    Cc: stable@kernel.org (3.0.x)
    Signed-off-by: Linus Torvalds

    Al Viro
     

10 Sep, 2011

1 commit

  • Prior to 2.6.38 automount would not trigger on either stat(2) or
    lstat(2) on the automount point.

    After 2.6.38, with the introduction of the ->d_automount()
    infrastructure, stat(2) and others would start triggering automount
    while lstat(2), etc. still would not. This is a regression and a
    userspace ABI change.

    Problem originally reported here:

    http://thread.gmane.org/gmane.linux.kernel.autofs/6098

    It appears that there was an attempt at fixing various userspace tools
    to not trigger the automount. But since the stat system call is
    rather common it is impossible to "fix" all userspace.

    This patch reverts the original behavior, which is to not trigger on
    stat(2) and other symlink following syscalls.

    [ It's not really clear what the right behavior is. Apparently Solaris
    does the "automount on stat, leave alone on lstat". And some programs
    can get unhappy when "stat+open+fstat" ends up giving a different
    result from the fstat than from the initial stat.

    But the change in 2.6.38 resulted in problems for some people, so
    we're going back to old behavior. Maybe we can re-visit this
    discussion at some future date - Linus ]

    Reported-by: Leonardo Chiquitto
    Signed-off-by: Miklos Szeredi
    Acked-by: Ian Kent
    Cc: David Howells
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Miklos Szeredi
     

08 Aug, 2011

1 commit


07 Aug, 2011

2 commits

  • After commit 3567866bf261: "RCUify freeing acls, let check_acl() go ahead in
    RCU mode if acl is cached" posix_acl_permission is being called with an
    unsupported flag and the permission check fails. This patch fixes the issue.

    Signed-off-by: Ari Savolainen
    Signed-off-by: Al Viro

    Ari Savolainen
     
  • The inode structure layout is largely random, and some of the vfs paths
    really do care. The path lookup in particular is already quite D$
    intensive, and profiles show that accessing the 'inode->i_op->xyz'
    fields is quite costly.

    We already optimized the dcache to not unnecessarily load the d_op
    structure for members that are often NULL using the DCACHE_OP_xyz bits
    in dentry->d_flags, and this does something very similar for the inode
    ops that are used during pathname lookup.

    It also re-orders the fields so that the fields accessed by 'stat' are
    together at the beginning of the inode structure, and roughly in the
    order accessed.

    The effect of this seems to be in the 1-2% range for an empty kernel
    "make -j" run (which is fairly kernel-intensive, mostly in filename
    lookup), so it's visible. The numbers are fairly noisy, though, and
    likely depend a lot on exact microarchitecture. So there's more tuning
    to be done.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

03 Aug, 2011

1 commit


01 Aug, 2011

1 commit


26 Jul, 2011

4 commits

  • Commit e77819e57f08 ("vfs: move ACL cache lookup into generic code")
    didn't take the FS_POSIX_ACL config variable into account - when that is
    not set, ACL's go away, and the cache helper functions do not exist,
    causing compile errors like

    fs/namei.c: In function 'check_acl':
    fs/namei.c:191:10: error: implicit declaration of function 'negative_cached_acl'
    fs/namei.c:196:2: error: implicit declaration of function 'get_cached_acl'
    fs/namei.c:196:6: warning: assignment makes pointer from integer without a cast
    fs/namei.c:212:11: error: implicit declaration of function 'set_cached_acl'

    Reported-by: Markus Trippelsdorf
    Acked-by: Stephen Rothwell
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • The "fsuid is the inode owner" case is not necessarily always the likely
    case, but it's the case that doesn't do anything odd and that we want in
    straight-line code. Make gcc not generate random "jump around for the
    fun of it" code.

    This just helps me read profiles. That thing is one of the hottest
    parts of the whole pathname lookup.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • Replace the ->check_acl method with a ->get_acl method that simply reads an
    ACL from disk after having a cache miss. This means we can replace the ACL
    checking boilerplate code with a single implementation in namei.c.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Al Viro

    Christoph Hellwig
     
  • This moves logic for checking the cached ACL values from low-level
    filesystems into generic code. The end result is a streamlined ACL
    check that doesn't need to load the inode->i_op->check_acl pointer at
    all for the common cached case.

    The filesystems also don't need to check for a non-blocking RCU walk
    case in their acl_check() functions, because that is all handled at a
    VFS layer.

    Signed-off-by: Linus Torvalds
    Signed-off-by: Al Viro

    Linus Torvalds
     

21 Jul, 2011

1 commit


20 Jul, 2011

9 commits