On Oct 17, 2016, at 4:27 AM, Alexey Lyashkov <alexey.lyashkov@xxxxxxxxx> wrote: > > Guys, > > While Andrew discovered a bug with non atomic d_move() usage in rename patch > (http://marc.info/?l=linux-fsdevel&m=147584842303074&w=2), I make a patch > which solve a problem. May you review it? > > Before LRU lookup landing cwd vs d_move race was resolved via dcache_lock, > but now, dentry may move into invalid state while rehashed due non atomic > dentry->d_hash changes. Patch tries to solve it problem and Andrew > reproducer run a hours now without error. It would be useful to include the test program in the commit message here, so that this can be tested more easily in the future. Also useful would be a test case for this in xfstests to avoid regressions in the future. > Reported-by: Andrew Perepechko <anserper@xxxxxxxxx> > Signed-off-by: Alexey Lyashkov <alexey.lyashkov@xxxxxxxxx> > > diff --git a/fs/dcache.c b/fs/dcache.c > index 5c7cc95..86a65d2 100644 > --- a/fs/dcache.c > +++ b/fs/dcache.c > @@ -438,7 +438,7 @@ static void dentry_lru_add(struct dentry *dentry) > */ > void __d_drop(struct dentry *dentry) > { > - if (!d_unhashed(dentry)) { > + if (!hlist_bl_unhashed(&dentry->d_hash)) { Rather than replacing d_unhashed() with raw hlist access of d_hash, it probably makes sense to keep a helper function like: static inline int __d_unhashed(const struct dentry *dentry) { return hlist_bl_unhashed(&dentry->d_hash); } and use that here, in __d_rehash(), and in the new d_unhashed(). Cheers, Andreas > struct hlist_bl_head *b; > /* > * Hashed dentries are normally on the dentry hashtable, > @@ -2342,7 +2342,8 @@ EXPORT_SYMBOL(d_delete); > static void __d_rehash(struct dentry *entry) > { > struct hlist_bl_head *b = d_hash(entry->d_name.hash); > - BUG_ON(!d_unhashed(entry)); > + > + BUG_ON(!hlist_bl_unhashed(&entry->d_hash)); > hlist_bl_lock(b); > hlist_bl_add_head_rcu(&entry->d_hash, b); > hlist_bl_unlock(b); > diff --git a/include/linux/dcache.h b/include/linux/dcache.h > index 5beed7b..83ba074 100644 > --- a/include/linux/dcache.h > +++ b/include/linux/dcache.h > @@ -330,7 +330,15 @@ extern struct dentry *dget_parent(struct dentry *dentry); > > static inline int d_unhashed(const struct dentry *dentry) > { > - return hlist_bl_unhashed(&dentry->d_hash); > + int ret; > + unsigned int seq; > + > + do { > + seq = read_seqcount_begin(&dentry->d_seq); > + ret = hlist_bl_unhashed(&dentry->d_hash); > + } while (read_seqcount_retry(&dentry->d_seq, seq)); > + > + return ret; > } Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
