On Tue, May 14, 2013 at 04:43:04PM +0400, Glauber Costa wrote:
> 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.
Right. Did I mention I was seeing a panic on a later patch? That's
because this patch removed the DCACHE_SHRINK_LIST check. So, the
DCACHE_SHRINK_LIST needs to move into dentry_lru_del(), and
dentry_lru_prune() can still go away. That's what my current version
of this patch does - I just didn't post it last night because it was
still running tests when I went to bed....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
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
