1. 08 8月, 2014 7 次提交
  2. 12 6月, 2014 1 次提交
    • A
      lock_parent: don't step on stale ->d_parent of all-but-freed one · c2338f2d
      Al Viro 提交于
      Dentry that had been through (or into) __dentry_kill() might be seen
      by shrink_dentry_list(); that's normal, it'll be taken off the shrink
      list and freed if __dentry_kill() has already finished.  The problem
      is, its ->d_parent might be pointing to already freed dentry, so
      lock_parent() needs to be careful.
      
      We need to check that dentry hasn't already gone into __dentry_kill()
      *and* grab rcu_read_lock() before dropping ->d_lock - the latter makes
      sure that whatever we see in ->d_parent after dropping ->d_lock it
      won't be freed until we drop rcu_read_lock().
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      c2338f2d
  3. 07 6月, 2014 1 次提交
  4. 01 6月, 2014 1 次提交
    • L
      dcache: add missing lockdep annotation · 9f12600f
      Linus Torvalds 提交于
      lock_parent() very much on purpose does nested locking of dentries, and
      is careful to maintain the right order (lock parent first).  But because
      it didn't annotate the nested locking order, lockdep thought it might be
      a deadlock on d_lock, and complained.
      
      Add the proper annotation for the inner locking of the child dentry to
      make lockdep happy.
      
      Introduced by commit 046b961b ("shrink_dentry_list(): take parent's
      ->d_lock earlier").
      Reported-and-tested-by: NJosh Boyer <jwboyer@fedoraproject.org>
      Cc: Al Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
      9f12600f
  5. 30 5月, 2014 3 次提交
    • A
      dentry_kill() doesn't need the second argument now · 8cbf74da
      Al Viro 提交于
      it's 1 in the only remaining caller.
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      8cbf74da
    • A
      dealing with the rest of shrink_dentry_list() livelock · b2b80195
      Al Viro 提交于
      We have the same problem with ->d_lock order in the inner loop, where
      we are dropping references to ancestors.  Same solution, basically -
      instead of using dentry_kill() we use lock_parent() (introduced in the
      previous commit) to get that lock in a safe way, recheck ->d_count
      (in case if lock_parent() has ended up dropping and retaking ->d_lock
      and somebody managed to grab a reference during that window), trylock
      the inode->i_lock and use __dentry_kill() to do the rest.
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      b2b80195
    • A
      shrink_dentry_list(): take parent's ->d_lock earlier · 046b961b
      Al Viro 提交于
      The cause of livelocks there is that we are taking ->d_lock on
      dentry and its parent in the wrong order, forcing us to use
      trylock on the parent's one.  d_walk() takes them in the right
      order, and unfortunately it's not hard to create a situation
      when shrink_dentry_list() can't make progress since trylock
      keeps failing, and shrink_dcache_parent() or check_submounts_and_drop()
      keeps calling d_walk() disrupting the very shrink_dentry_list() it's
      waiting for.
      
      Solution is straightforward - if that trylock fails, let's unlock
      the dentry itself and take locks in the right order.  We need to
      stabilize ->d_parent without holding ->d_lock, but that's doable
      using RCU.  And we'd better do that in the very beginning of the
      loop in shrink_dentry_list(), since the checks on refcount, etc.
      would need to be redone anyway.
      
      That deals with a half of the problem - killing dentries on the
      shrink list itself.  Another one (dropping their parents) is
      in the next commit.
      
      locking parent is interesting - it would be easy to do rcu_read_lock(),
      lock whatever we think is a parent, lock dentry itself and check
      if the parent is still the right one.  Except that we need to check
      that *before* locking the dentry, or we are risking taking ->d_lock
      out of order.  Fortunately, once the D1 is locked, we can check if
      D2->d_parent is equal to D1 without the need to lock D2; D2->d_parent
      can start or stop pointing to D1 only under D1->d_lock, so taking
      D1->d_lock is enough.  In other words, the right solution is
      rcu_read_lock/lock what looks like parent right now/check if it's
      still our parent/rcu_read_unlock/lock the child.
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      046b961b
  6. 29 5月, 2014 2 次提交
  7. 28 5月, 2014 1 次提交
  8. 04 5月, 2014 3 次提交
    • M
      dcache: don't need rcu in shrink_dentry_list() · 60942f2f
      Miklos Szeredi 提交于
      Since now the shrink list is private and nobody can free the dentry while
      it is on the shrink list, we can remove RCU protection from this.
      Signed-off-by: NMiklos Szeredi <mszeredi@suse.cz>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      60942f2f
    • A
      more graceful recovery in umount_collect() · 9c8c10e2
      Al Viro 提交于
      Start with shrink_dcache_parent(), then scan what remains.
      
      First of all, BUG() is very much an overkill here; we are holding
      ->s_umount, and hitting BUG() means that a lot of interesting stuff
      will be hanging after that point (sync(2), for example).  Moreover,
      in cases when there had been more than one leak, we'll be better
      off reporting all of them.  And more than just the last component
      of pathname - %pd is there for just such uses...
      
      That was the last user of dentry_lru_del(), so kill it off...
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      9c8c10e2
    • A
      don't remove from shrink list in select_collect() · fe91522a
      Al Viro 提交于
      	If we find something already on a shrink list, just increment
      data->found and do nothing else.  Loops in shrink_dcache_parent() and
      check_submounts_and_drop() will do the right thing - everything we
      did put into our list will be evicted and if there had been nothing,
      but data->found got non-zero, well, we have somebody else shrinking
      those guys; just try again.
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      fe91522a
  9. 01 5月, 2014 5 次提交
  10. 20 4月, 2014 1 次提交
    • A
      fix races between __d_instantiate() and checks of dentry flags · 22213318
      Al Viro 提交于
      in non-lazy walk we need to be careful about dentry switching from
      negative to positive - both ->d_flags and ->d_inode are updated,
      and in some places we might see only one store.  The cases where
      dentry has been obtained by dcache lookup with ->i_mutex held on
      parent are safe - ->d_lock and ->i_mutex provide all the barriers
      we need.  However, there are several places where we run into
      trouble:
      	* do_last() fetches ->d_inode, then checks ->d_flags and
      assumes that inode won't be NULL unless d_is_negative() is true.
      Race with e.g. creat() - we might have fetched the old value of
      ->d_inode (still NULL) and new value of ->d_flags (already not
      DCACHE_MISS_TYPE).  Lin Ming has observed and reported the resulting
      oops.
      	* a bunch of places checks ->d_inode for being non-NULL,
      then checks ->d_flags for "is it a symlink".  Race with symlink(2)
      in case if our CPU sees ->d_inode update first - we see non-NULL
      there, but ->d_flags still contains DCACHE_MISS_TYPE instead of
      DCACHE_SYMLINK_TYPE.  Result: false negative on "should we follow
      link here?", with subsequent unpleasantness.
      
      Cc: stable@vger.kernel.org # 3.13 and 3.14 need that one
      Reported-and-tested-by: NLin Ming <minggr@gmail.com>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      22213318
  11. 01 4月, 2014 1 次提交
  12. 23 3月, 2014 1 次提交
  13. 16 3月, 2014 1 次提交
    • D
      drm: add pseudo filesystem for shared inodes · 31bbe16f
      David Herrmann 提交于
      Our current DRM design uses a single address_space for all users of the
      same DRM device. However, there is no way to create an anonymous
      address_space without an underlying inode. Therefore, we wait for the
      first ->open() callback on a registered char-dev and take-over the inode
      of the char-dev. This worked well so far, but has several drawbacks:
       - We screw with FS internals and rely on some non-obvious invariants like
         inode->i_mapping being the same as inode->i_data for char-devs.
       - We don't have any address_space prior to the first ->open() from
         user-space. This leads to ugly fallback code and we cannot allocate
         global objects early.
      
      As pointed out by Al-Viro, fs/anon_inode.c is *not* supposed to be used by
      drivers for anonymous inode-allocation. Therefore, this patch follows the
      proposed alternative solution and adds a pseudo filesystem mount-point to
      DRM. We can then allocate private inodes including a private address_space
      for each DRM device at initialization time.
      
      Note that we could use:
        sysfs_get_inode(sysfs_mnt->mnt_sb, drm_device->dev->kobj.sd);
      to get access to the underlying sysfs-inode of a "struct device" object.
      However, most of this information is currently hidden and it's not clear
      whether this address_space is suitable for driver access. Thus, unless
      linux allows anonymous address_space objects or driver-core provides a
      public inode per device, we're left with our own private internal mount
      point.
      
      Cc: Al Viro <viro@zeniv.linux.org.uk>
      Signed-off-by: NDavid Herrmann <dh.herrmann@gmail.com>
      31bbe16f
  14. 27 1月, 2014 1 次提交
  15. 26 1月, 2014 1 次提交
    • E
      vfs: Remove second variable named error in __dentry_path · a8323da0
      Eric W. Biederman 提交于
      In commit  232d2d60
      Author: Waiman Long <Waiman.Long@hp.com>
      Date:   Mon Sep 9 12:18:13 2013 -0400
      
          dcache: Translating dentry into pathname without taking rename_lock
      
      The __dentry_path locking was changed and the variable error was
      intended to be moved outside of the loop.  Unfortunately the inner
      declaration of error was not removed. Resulting in a version of
      __dentry_path that will never return an error.
      
      Remove the problematic inner declaration of error and allow
      __dentry_path to return errors once again.
      
      Cc: stable@vger.kernel.org
      Cc: Waiman Long <Waiman.Long@hp.com>
      Signed-off-by: N"Eric W. Biederman" <ebiederm@xmission.com>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      a8323da0
  16. 13 12月, 2013 1 次提交
  17. 27 11月, 2013 1 次提交
    • E
      vfs: In d_path don't call d_dname on a mount point · f48cfddc
      Eric W. Biederman 提交于
      Aditya Kali (adityakali@google.com) wrote:
      > Commit bf056bfa:
      > "proc: Fix the namespace inode permission checks." converted
      > the namespace files into symlinks. The same commit changed
      > the way namespace bind mounts appear in /proc/mounts:
      >   $ mount --bind /proc/self/ns/ipc /mnt/ipc
      > Originally:
      >   $ cat /proc/mounts | grep ipc
      >   proc /mnt/ipc proc rw,nosuid,nodev,noexec 0 0
      >
      > After commit bf056bfa:
      >   $ cat /proc/mounts | grep ipc
      >   proc ipc:[4026531839] proc rw,nosuid,nodev,noexec 0 0
      >
      > This breaks userspace which expects the 2nd field in
      > /proc/mounts to be a valid path.
      
      The symlink /proc/<pid>/ns/{ipc,mnt,net,pid,user,uts} point to
      dentries allocated with d_alloc_pseudo that we can mount, and
      that have interesting names printed out with d_dname.
      
      When these files are bind mounted /proc/mounts is not currently
      displaying the mount point correctly because d_dname is called instead
      of just displaying the path where the file is mounted.
      
      Solve this by adding an explicit check to distinguish mounted pseudo
      inodes and unmounted pseudo inodes.  Unmounted pseudo inodes always
      use mount of their filesstem as the mnt_root  in their path making
      these two cases easy to distinguish.
      
      CC: stable@vger.kernel.org
      Acked-by: NSerge Hallyn <serge.hallyn@canonical.com>
      Reported-by: NAditya Kali <adityakali@google.com>
      Signed-off-by: N"Eric W. Biederman" <ebiederm@xmission.com>
      f48cfddc
  18. 16 11月, 2013 3 次提交
  19. 13 11月, 2013 2 次提交
  20. 09 11月, 2013 3 次提交
    • J
      dcache: don't clear DCACHE_DISCONNECTED too early · f80de2cd
      J. Bruce Fields 提交于
      DCACHE_DISCONNECTED should not be cleared until we're sure the dentry is
      connected all the way up to the root of the filesystem.  It *shouldn't*
      be cleared as soon as the dentry is connected to a parent.  That will
      cause bugs at least on exportable filesystems.
      Acked-by: NChristoph Hellwig <hch@infradead.org>
      Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      f80de2cd
    • J
      dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries · e1a24bb0
      J. Bruce Fields 提交于
      I can't for the life of me see any reason why anyone should care whether
      a dentry that is never hooked into the dentry cache would need
      DCACHE_DISCONNECTED set.
      
      This originates from 4b936885 "fs:
      improve scalability of pseudo filesystems", which probably just made the
      false assumption the DCACHE_DISCONNECTED was meant to be set on anything
      not connected to a parent somehow.
      
      So this is just confusing.  Ideally the only uses of DCACHE_DISCONNECTED
      would be in the filehandle-lookup code, which needs it to ensure
      dentries are connected into the dentry tree before use.
      
      I left d_alloc_pseudo there even though it's now equivalent to
      __d_alloc(), just on the theory the name is better documentation of its
      intended use outside dcache.c.
      
      Cc: Nick Piggin <npiggin@kernel.dk>
      Acked-by: NChristoph Hellwig <hch@infradead.org>
      Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      e1a24bb0
    • J
      dcache: use IS_ROOT to decide where dentry is hashed · 7632e465
      J. Bruce Fields 提交于
      Every hashed dentry is either hashed in the dentry_hashtable, or a
      superblock's s_anon list.
      
      __d_drop() assumes it can determine which is the case by checking
      DCACHE_DISCONNECTED; this is not true.
      
      It is true that when DCACHE_DISCONNECTED is cleared, the dentry is not
      only hashed on dentry_hashtable, but is fully connected to its parents
      back to the root.
      
      But the converse is *not* true: fs/exportfs/expfs.c:reconnect_path()
      attempts to connect a directory (found by filehandle lookup) back to
      root by ascending to parents and performing lookups one at a time.  It
      does not clear DCACHE_DISCONNECTED until it's done, and that is not at
      all an atomic process.
      
      In particular, it is possible for DCACHE_DISCONNECTED to be set on a
      dentry which is hashed on the dentry_hashtable.
      
      Instead, use IS_ROOT() to check which hash chain a dentry is on.  This
      *does* work:
      
      Dentries are hashed only by:
      
      	- d_obtain_alias, which adds an IS_ROOT() dentry to sb_anon.
      
      	- __d_rehash, called by _d_rehash: hashes to the dentry's
      	  parent, and all callers of _d_rehash appear to have d_parent
      	  set to a "real" parent.
      	- __d_rehash, called by __d_move: rehashes the moved dentry to
      	  hash chain determined by target, and assigns target's d_parent
      	  to its d_parent, before dropping the dentry's d_lock.
      
      Therefore I believe it's safe for a holder of a dentry's d_lock to
      assume that it is hashed on sb_anon if and only if IS_ROOT(dentry) is
      true.
      
      I believe the incorrect assumption about DCACHE_DISCONNECTED was
      originally introduced by ceb5bdc2 "fs: dcache per-bucket dcache hash
      locking".
      
      Also add a comment while we're here.
      
      Cc: Nick Piggin <npiggin@kernel.dk>
      Acked-by: NChristoph Hellwig <hch@infradead.org>
      Reviewed-by: NNeilBrown <neilb@suse.de>
      Signed-off-by: NJ. Bruce Fields <bfields@redhat.com>
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      7632e465