On 05/14/2013 09:46 AM, Dave Chinner wrote:
> [ v2: don't decrement nr unused twice, spotted by Sha Zhengju ]
> [ v7: (dchinner)
> - shrink list leaks dentries when inode/parent can't be locked in
> dentry_kill().
> - fix the scope of the sb locking inside shrink_dcache_sb()
> - remove the readdition of dentry_lru_prune(). ]
Dave,
dentry_lru_prune was removed because it would only prune the dentry if
it was in the LRU list, and it has to be always pruned (61572bb1).
You don't reintroduce dentry_lru_prune here, so the two locations which
prune dentries read as follows:
if (dentry->d_flags & DCACHE_OP_PRUNE)
dentry->d_op->d_prune(dentry);
dentry_lru_del(dentry);
I believe this is wrong. My old version would do:
+static void dentry_lru_prune(struct dentry *dentry)
+{
+ /*
+ * inform the fs via d_prune that this dentry is about to be
+ * unhashed and destroyed.
+ */
+ if (dentry->d_flags & DCACHE_OP_PRUNE)
+ dentry->d_op->d_prune(dentry);
+
+ if (list_empty(&dentry->d_lru))
+ return;
+
+ if ((dentry->d_flags & DCACHE_SHRINK_LIST)) {
+ list_del_init(&dentry->d_lru);
+ dentry->d_flags &= ~DCACHE_SHRINK_LIST;
+ } else {
+ spin_lock(&dentry->d_sb->s_dentry_lru_lock);
+ __dentry_lru_del(dentry);
+ spin_unlock(&dentry->d_sb->s_dentry_lru_lock);
+ }
+}
Which is SHRINK_LIST aware. The code as it reads today after your patch
will only be correct if it is totally impossible for a dentry to be in
the shrink list before we reach both sites that call d_op->d_prune. They
are: dentry_kill and shrink_dcache_for_umount_subtree. So it seems to me
that we do really need to reintroduce dentry_lru_prune or just patch
both call sites with shrik-list aware code.
Comments ?
--
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
