19 Mar, 2011

2 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6:
    fs: call security_d_instantiate in d_obtain_alias V2
    lose 'mounting_here' argument in ->d_manage()
    don't pass 'mounting_here' flag to follow_down()
    change the locking order for namespace_sem
    fix deadlock in pivot_root()
    vfs: split off vfsmount-related parts of vfs_kern_mount()
    Some fixes for pstore
    kill simple_set_mnt()

    Linus Torvalds
     
  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (47 commits)
    doc: CONFIG_UNEVICTABLE_LRU doesn't exist anymore
    Update cpuset info & webiste for cgroups
    dcdbas: force SMI to happen when expected
    arch/arm/Kconfig: remove one to many l's in the word.
    asm-generic/user.h: Fix spelling in comment
    drm: fix printk typo 'sracth'
    Remove one to many n's in a word
    Documentation/filesystems/romfs.txt: fixing link to genromfs
    drivers:scsi Change printk typo initate -> initiate
    serial, pch uart: Remove duplicate inclusion of linux/pci.h header
    fs/eventpoll.c: fix spelling
    mm: Fix out-of-date comments which refers non-existent functions
    drm: Fix printk typo 'failled'
    coh901318.c: Change initate to initiate.
    mbox-db5500.c Change initate to initiate.
    edac: correct i82975x error-info reported
    edac: correct i82975x mci initialisation
    edac: correct commented info
    fs: update comments to point correct document
    target: remove duplicate include of target/target_core_device.h from drivers/target/target_core_hba.c
    ...

    Trivial conflict in fs/eventpoll.c (spelling vs addition)

    Linus Torvalds
     

18 Mar, 2011

1 commit

  • While trying to track down some NFS problems with BTRFS, I kept noticing I was
    getting -EACCESS for no apparent reason. Eric Paris and printk() helped me
    figure out that it was SELinux that was giving me grief, with the following
    denial

    type=AVC msg=audit(1290013638.413:95): avc: denied { 0x800000 } for pid=1772
    comm="nfsd" name="" dev=sda1 ino=256 scontext=system_u:system_r:kernel_t:s0
    tcontext=system_u:object_r:unlabeled_t:s0 tclass=file

    Turns out this is because in d_obtain_alias if we can't find an alias we create
    one and do all the normal instantiation stuff, but we don't do the
    security_d_instantiate.

    Usually we are protected from getting a hashed dentry that hasn't yet run
    security_d_instantiate() by the parent's i_mutex, but obviously this isn't an
    option there, so in order to deal with the case that a second thread comes in
    and finds our new dentry before we get to run security_d_instantiate(), we go
    ahead and call it if we find a dentry already. Eric assures me that this is ok
    as the code checks to see if the dentry has been initialized already so calling
    security_d_instantiate() against the same dentry multiple times is ok. With
    this patch I'm no longer getting errant -EACCESS values.

    Signed-off-by: Josef Bacik
    Signed-off-by: Al Viro

    Josef Bacik
     

16 Mar, 2011

2 commits

  • The new vfs locking scheme introduced in 2.6.38 breaks NFS sillyrename
    because the latter relies on being able to determine the parent
    directory of the dentry in the ->iput() callback in order to send the
    appropriate unlink rpc call.

    Looking at the code that cares about races with dput(), there doesn't
    seem to be anything that specifically uses d_parent as a test for
    whether or not there is a race:
    - __d_lookup_rcu(), __d_lookup() all test for d_hashed() after d_parent
    - shrink_dcache_for_umount() is safe since nothing else can rearrange
    the dentries in that super block.
    - have_submount(), select_parent() and d_genocide() can test for a
    deletion if we set the DCACHE_DISCONNECTED flag when the dentry
    is removed from the parent's d_subdirs list.

    Signed-off-by: Trond Myklebust
    Cc: stable@kernel.org (2.6.38, needs commit c826cb7dfce8 "dcache.c:
    create helper function for duplicated functionality" )
    Signed-off-by: Linus Torvalds

    Trond Myklebust
     
  • This creates a helper function for he "try to ascend into the parent
    directory" case, which was written out in triplicate before. With all
    the locking and subtle sequence number stuff, we really don't want to
    duplicate that kind of code.

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

10 Mar, 2011

1 commit

  • Without this patch, inodes are not promptly freed on last close of an
    unlinked file by an nfs client:

    client$ mount -tnfs4 server:/export/ /mnt/
    client$ tail -f /mnt/FOO
    ...
    server$ df -i /export
    server$ rm /export/FOO
    (^C the tail -f)
    server$ df -i /export
    server$ echo 2 >/proc/sys/vm/drop_caches
    server$ df -i /export

    the df's will show that the inode is not freed on the filesystem until
    the last step, when it could have been freed after killing the client's
    tail -f. On-disk data won't be deallocated either, leading to possible
    spurious ENOSPC.

    This occurs because when the client does the close, it arrives in a
    compound with a putfh and a close, processed like:

    - putfh: look up the filehandle.  The only alias found for the
    inode will be DCACHE_UNHASHED alias referenced by the filp
    this, so it creates a new DCACHE_DISCONECTED dentry and
    returns that instead.
    - close: closes the existing filp, which is destroyed
    immediately by dput() since it's DCACHE_UNHASHED.
    - end of the compound: release the reference
    to the current filehandle, and dput() the new
    DCACHE_DISCONECTED dentry, which gets put on the
    unused list instead of being destroyed immediately.

    Nick Piggin suggested fixing this by allowing d_obtain_alias to return
    the unhashed dentry that is referenced by the filp, instead of making it
    create a new dentry.

    Leave __d_find_alias() alone to avoid changing behavior of other
    callers.

    Also nfsd doesn't need all the checks of __d_find_alias(); any dentry,
    hashed or unhashed, disconnected or not, should work.

    Signed-off-by: J. Bruce Fields
    Signed-off-by: Al Viro

    J. Bruce Fields
     

17 Feb, 2011

1 commit


23 Jan, 2011

1 commit

  • Fix new fs/dcache.c kernel-doc warnings:

    Warning(fs/dcache.c:184): No description found for parameter 'dentry'
    Warning(fs/dcache.c:296): No description found for parameter 'parent'
    Warning(fs/dcache.c:1985): No description found for parameter 'dparent'
    Warning(fs/dcache.c:1985): Excess function parameter 'parent' description in 'd_validate'

    Signed-off-by: Randy Dunlap
    Cc: Alexander Viro
    Cc: Nick Piggin
    Signed-off-by: Linus Torvalds

    Randy Dunlap
     

17 Jan, 2011

1 commit

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (23 commits)
    sanitize vfsmount refcounting changes
    fix old umount_tree() breakage
    autofs4: Merge the remaining dentry ops tables
    Unexport do_add_mount() and add in follow_automount(), not ->d_automount()
    Allow d_manage() to be used in RCU-walk mode
    Remove a further kludge from __do_follow_link()
    autofs4: Bump version
    autofs4: Add v4 pseudo direct mount support
    autofs4: Fix wait validation
    autofs4: Clean up autofs4_free_ino()
    autofs4: Clean up dentry operations
    autofs4: Clean up inode operations
    autofs4: Remove unused code
    autofs4: Add d_manage() dentry operation
    autofs4: Add d_automount() dentry operation
    Remove the automount through follow_link() kludge code from pathwalk
    CIFS: Use d_automount() rather than abusing follow_link()
    NFS: Use d_automount() rather than abusing follow_link()
    AFS: Use d_automount() rather than abusing follow_link()
    Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
    ...

    Linus Torvalds
     

16 Jan, 2011

1 commit

  • Add a dentry op (d_automount) to handle automounting directories rather than
    abusing the follow_link() inode operation. The operation is keyed off a new
    dentry flag (DCACHE_NEED_AUTOMOUNT).

    This also makes it easier to add an AT_ flag to suppress terminal segment
    automount during pathwalk and removes the need for the kludge code in the
    pathwalk algorithm to handle directories with follow_link() semantics.

    The ->d_automount() dentry operation:

    struct vfsmount *(*d_automount)(struct path *mountpoint);

    takes a pointer to the directory to be mounted upon, which is expected to
    provide sufficient data to determine what should be mounted. If successful, it
    should return the vfsmount struct it creates (which it should also have added
    to the namespace using do_add_mount() or similar). If there's a collision with
    another automount attempt, NULL should be returned. If the directory specified
    by the parameter should be used directly rather than being mounted upon,
    -EISDIR should be returned. In any other case, an error code should be
    returned.

    The ->d_automount() operation is called with no locks held and may sleep. At
    this point the pathwalk algorithm will be in ref-walk mode.

    Within fs/namei.c itself, a new pathwalk subroutine (follow_automount()) is
    added to handle mountpoints. It will return -EREMOTE if the automount flag was
    set, but no d_automount() op was supplied, -ELOOP if we've encountered too many
    symlinks or mountpoints, -EISDIR if the walk point should be used without
    mounting and 0 if successful. The path will be updated to point to the mounted
    filesystem if a successful automount took place.

    __follow_mount() is replaced by follow_managed() which is more generic
    (especially with the patch that adds ->d_manage()). This handles transits from
    directories during pathwalk, including automounting and skipping over
    mountpoints (and holding processes with the next patch).

    __follow_mount_rcu() will jump out of RCU-walk mode if it encounters an
    automount point with nothing mounted on it.

    follow_dotdot*() does not handle automounts as you don't want to trigger them
    whilst following "..".

    I've also extracted the mount/don't-mount logic from autofs4 and included it
    here. It makes the mount go ahead anyway if someone calls open() or creat(),
    tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
    or sticks a '/' on the end of the pathname. If they do a stat(), however,
    they'll only trigger the automount if they didn't also say O_NOFOLLOW.

    I've also added an inode flag (S_AUTOMOUNT) so that filesystems can mark their
    inodes as automount points. This flag is automatically propagated to the
    dentry as DCACHE_NEED_AUTOMOUNT by __d_instantiate(). This saves NFS and could
    save AFS a private flag bit apiece, but is not strictly necessary. It would be
    preferable to do the propagation in d_set_d_op(), but that doesn't normally
    have access to the inode.

    [AV: fixed breakage in case if __follow_mount_rcu() fails and nameidata_drop_rcu()
    succeeds in RCU case of do_lookup(); we need to fall through to non-RCU case after
    that, rather than just returning with ungrabbed *path]

    Signed-off-by: David Howells
    Was-Acked-by: Ian Kent
    Signed-off-by: Al Viro

    David Howells
     

15 Jan, 2011

1 commit

  • It's indicative of a real problem, and it actually triggers with
    autofs4, but the BUG_ON() is excessive. The autofs4 case is being fixed
    (to only set d_op in the ->lookup method) but not merged yet. In the
    meantime this gets the code limping along.

    Reported-by: Alex Elder
    Cc: Ian Kent
    Cc: Nick Piggin
    Cc: Al Viro
    Signed-off-by: Linus Torvalds

    Linus Torvalds
     

13 Jan, 2011

3 commits


07 Jan, 2011

26 commits

  • The standard memcmp function on a Westmere system shows up hot in
    profiles in the `git diff` workload (both parallel and single threaded),
    and it is likely due to the costs associated with trapping into
    microcode, and little opportunity to improve memory access (dentry
    name is not likely to take up more than a cacheline).

    So replace it with an open-coded byte comparison. This increases code
    size by 8 bytes in the critical __d_lookup_rcu function, but the
    speedup is huge, averaging 10 runs of each:

    git diff st user sys elapsed CPU
    before 1.15 2.57 3.82 97.1
    after 1.14 2.35 3.61 96.8

    git diff mt user sys elapsed CPU
    before 1.27 3.85 1.46 349
    after 1.26 3.54 1.43 333

    Elapsed time for single threaded git diff at 95.0% confidence:
    -0.21 +/- 0.01
    -5.45% +/- 0.24%

    It's -0.66% +/- 0.06% elapsed time on my Opteron, so rep cmp costs on the
    fam10h seem to be relatively smaller, but there is still a win.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • This makes single threaded git diff -1.25% +/- 0.05% elapsed time on my
    2s12c24t Westmere system, and -0.86% +/- 0.05% on my 2s8c Barcelona, by
    prefetching the important first cacheline of the inode in while we do the
    actual name compare and other operations on the dentry.

    There was no measurable slowdown in the single file stat case, or the creat
    case (where negative dentries would be common).

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Regardless of how much we possibly try to scale dcache, there is likely
    always going to be some fundamental contention when adding or removing children
    under the same parent. Pseudo filesystems do not seem need to have connected
    dentries because by definition they are disconnected.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • dcache_inode_lock can be replaced with per-inode locking. Use existing
    inode->i_lock for this. This is slightly non-trivial because we sometimes
    need to find the inode from the dentry, which requires d_inode to be
    stabilised (either with refcount or d_lock).

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • We can turn the dcache hash locking from a global dcache_hash_lock into
    per-bucket locking.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Put dentry and inode fields into top of data structure. This allows RCU path
    traversal to perform an RCU dentry lookup in a path walk by touching only the
    first 56 bytes of the dentry.

    We also fit in 8 bytes of inline name in the first 64 bytes, so for short
    names, only 64 bytes needs to be touched to perform the lookup. We should
    get rid of the hash->prev pointer from the first 64 bytes, and fit 16 bytes
    of name in there, which will take care of 81% rather than 32% of the kernel
    tree.

    inode is also rearranged so that RCU lookup will only touch a single cacheline
    in the inode, plus one in the i_ops structure.

    This is important for directory component lookups in RCU path walking. In the
    kernel source, directory names average is around 6 chars, so this works.

    When we reach the last element of the lookup, we need to lock it and take its
    refcount which requires another cacheline access.

    Align dentry and inode operations structs, so members will be at predictable
    offsets and we can group common operations into head of structure.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Reduce some branches and memory accesses in dcache lookup by adding dentry
    flags to indicate common d_ops are set, rather than having to check them.
    This saves a pointer memory access (dentry->d_op) in common path lookup
    situations, and saves another pointer load and branch in cases where we
    have d_op but not the particular operation.

    Patched with:

    git grep -E '[.>]([[:space:]])*d_op([[:space:]])*=' | xargs sed -e 's/\([^\t ]*\)->d_op = \(.*\);/d_set_d_op(\1, \2);/' -e 's/\([^\t ]*\)\.d_op = \(.*\);/d_set_d_op(\&\1, \2);/' -i

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Rather than keep a d_mounted count in the dentry, set a dentry flag instead.
    The flag can be cleared by checking the hash table to see if there are any
    mounts left, which is not time critical because it is performed at detach time.

    The mounted state of a dentry is only used to speculatively take a look in the
    mount hash table if it is set -- before following the mount, vfsmount lock is
    taken and mount re-checked without races.

    This saves 4 bytes on 32-bit, nothing on 64-bit but it does provide a hole I
    might use later (and some configs have larger than 32-bit spinlocks which might
    make use of the hole).

    Autofs4 conversion and changelog by Ian Kent :
    In autofs4, when expring direct (or offset) mounts we need to ensure that we
    block user path walks into the autofs mount, which is covered by another mount.
    To do this we clear the mounted status so that follows stop before walking into
    the mount and are essentially blocked until the expire is completed. The
    automount daemon still finds the correct dentry for the umount due to the
    follow mount logic in fs/autofs4/root.c:autofs4_follow_link(), which is set as
    an inode operation for direct and offset mounts only and is called following
    the lookup that stopped at the covered mount.

    At the end of the expire the covering mount probably has gone away so the
    mounted status need not be restored. But we need to check this and only restore
    the mounted status if the expire failed.

    XXX: autofs may not work right if we have other mounts go over the top of it?

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Perform common cases of path lookups without any stores or locking in the
    ancestor dentry elements. This is called rcu-walk, as opposed to the current
    algorithm which is a refcount based walk, or ref-walk.

    This results in far fewer atomic operations on every path element,
    significantly improving path lookup performance. It also avoids cacheline
    bouncing on common dentries, significantly improving scalability.

    The overall design is like this:
    * LOOKUP_RCU is set in nd->flags, which distinguishes rcu-walk from ref-walk.
    * Take the RCU lock for the entire path walk, starting with the acquiring
    of the starting path (eg. root/cwd/fd-path). So now dentry refcounts are
    not required for dentry persistence.
    * synchronize_rcu is called when unregistering a filesystem, so we can
    access d_ops and i_ops during rcu-walk.
    * Similarly take the vfsmount lock for the entire path walk. So now mnt
    refcounts are not required for persistence. Also we are free to perform mount
    lookups, and to assume dentry mount points and mount roots are stable up and
    down the path.
    * Have a per-dentry seqlock to protect the dentry name, parent, and inode,
    so we can load this tuple atomically, and also check whether any of its
    members have changed.
    * Dentry lookups (based on parent, candidate string tuple) recheck the parent
    sequence after the child is found in case anything changed in the parent
    during the path walk.
    * inode is also RCU protected so we can load d_inode and use the inode for
    limited things.
    * i_mode, i_uid, i_gid can be tested for exec permissions during path walk.
    * i_op can be loaded.

    When we reach the destination dentry, we lock it, recheck lookup sequence,
    and increment its refcount and mountpoint refcount. RCU and vfsmount locks
    are dropped. This is termed "dropping rcu-walk". If the dentry refcount does
    not match, we can not drop rcu-walk gracefully at the current point in the
    lokup, so instead return -ECHILD (for want of a better errno). This signals the
    path walking code to re-do the entire lookup with a ref-walk.

    Aside from the final dentry, there are other situations that may be encounted
    where we cannot continue rcu-walk. In that case, we drop rcu-walk (ie. take
    a reference on the last good dentry) and continue with a ref-walk. Again, if
    we can drop rcu-walk gracefully, we return -ECHILD and do the whole lookup
    using ref-walk. But it is very important that we can continue with ref-walk
    for most cases, particularly to avoid the overhead of double lookups, and to
    gain the scalability advantages on common path elements (like cwd and root).

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

    In future patches, permission checks and d_revalidate become rcu-walk aware. It
    may be possible eventually to make following links rcu-walk aware.

    Uncached path elements will always require dropping to ref-walk mode, at the
    very least because i_mutex needs to be grabbed, and objects allocated.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • The tricky locking for disposing of a dentry is duplicated 3 times in the
    dcache (dput, pruning a dentry from the LRU, and pruning its ancestors).
    Consolidate them all into a single function dentry_kill.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Signed-off-by: Nick Piggin

    Nick Piggin
     
  • prune_one_dentry can avoid quite a bit of locking in the common case where
    ancestors have an elevated refcount. Alternatively, we could have gone the
    other way and made fewer trylocks in the case where d_count goes to zero, but
    is probably less common.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Use RCU to simplify locking in dget_parent.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • dget_locked was a shortcut to avoid the lazy lru manipulation when we already
    held dcache_lock (lru manipulation was relatively cheap at that point).
    However, how that the lru lock is an innermost one, we never hold it at any
    caller, so the lock cost can now be avoided. We already have well working lazy
    dcache LRU, so it should be fine to defer LRU manipulations to scan time.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • dcache_inode_lock can be avoided in d_delete() and d_materialise_unique()
    in cases where it is not required.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Signed-off-by: Nick Piggin

    Nick Piggin
     
  • It is possible to run dput without taking data structure locks up-front. In
    many cases where we don't kill the dentry anyway, these locks are not required.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Long lived dcache "multi-step" operations which retry on rename seq can
    be starved with a lot of rename activity. If they fail after the 1st pass,
    take the rename_lock for writing to avoid further starvation.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • dcache_lock no longer protects anything. remove it.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • The remaining usages for dcache_lock is to allow atomic, multi-step read-side
    operations over the directory tree by excluding modifications to the tree.
    Also, to walk in the leaf->root direction in the tree where we don't have
    a natural d_lock ordering.

    This could be accomplished by taking every d_lock, but this would mean a
    huge number of locks and actually gets very tricky.

    Solve this instead by using the rename seqlock for multi-step read-side
    operations, retry in case of a rename so we don't walk up the wrong parent.
    Concurrent dentry insertions are not serialised against. Concurrent deletes
    are tricky when walking up the directory: our parent might have been deleted
    when dropping locks so also need to check and retry for that.

    We can also use the rename lock in cases where livelock is a worry (and it
    is introduced in subsequent patch).

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Cover d_name with d_lock in more cases, where there may be concurrent
    modification to it.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Add a new lock, dcache_inode_lock, to protect the inode's i_dentry list
    from concurrent modification. d_alias is also protected by d_lock.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Protect d_subdirs and d_child with d_lock, except in filesystems that aren't
    using dcache_lock for these anyway (eg. using i_mutex).

    Note: if we change the locking rule in future so that ->d_child protection is
    provided only with ->d_parent->d_lock, it may allow us to reduce some locking.
    But it would be an exception to an otherwise regular locking scheme, so we'd
    have to see some good results. Probably not worthwhile.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Protect d_unhashed(dentry) condition with d_lock. This means keeping
    DCACHE_UNHASHED bit in synch with hash manipulations.

    Signed-off-by: Nick Piggin

    Nick Piggin
     
  • Make d_count non-atomic and protect it with d_lock. This allows us to ensure a
    0 refcount dentry remains 0 without dcache_lock. It is also fairly natural when
    we start protecting many other dentry members with d_lock.

    Signed-off-by: Nick Piggin

    Nick Piggin