Re: [PATCH] dcache: Translating dentry into pathname without taking rename_lock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/04/2013 05:31 PM, Linus Torvalds wrote:
On Wed, Sep 4, 2013 at 12:05 PM, Waiman Long<Waiman.Long@xxxxxx>  wrote:
+       rcu_read_unlock();
+       if (read_seqretry(&rename_lock, seq))
+               goto restart;
Btw, you have this pattern twice, and while it's not necessarily
incorrect, it's a bit worrisome for performance.

The rcu_read_unlock sequence in the middle of prepend_path() is not likely to executed. So it shouldn't affect performance exception for the conditional check.

The rcu_read_unlock() is very possibly going to trigger an immediate
scheduling event, so checking the sequence lock after dropping the
read-lock sounds like it would make it much easier to hit the race
with some rename.

I can put read_seqbegin/read_seqretry within the rcu_read_lock/rcu_read_unlock block. However, read_seqbegin() can spin for a while if a rename is in progress. So it depends on which is more important, a shorter RCU critical section at the expense of more retries or vice versa.

I'm also a tiny bit worried about livelocking on the sequence lock in
the presence of lots of renames, so I'm wondering if the locking
should try to approximate what we do for the RCU lookup path: start
off optimistically using just the RCU lock and a sequence point, but
if that fails, fall back to taking the real lock. Maybe using a
counter ("try twice, then get the rename lock for writing")

Hmm?

Yes, I can implement a counter that switch to taking the rename_lock if the count reaches 0. It shouldn't be too hard. That should avoid the possibility of a livelock.

Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux