On Tue, May 14, 2013 at 03:46:40PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> One of the big problems with modifying the way the dcache shrinker
> and LRU implementation works is that the LRU is abused in several
> ways. One of these is shrink_dentry_list().
>
> Basically, we can move a dentry off the LRU onto a different list
> without doing any accounting changes, and then use dentry_lru_prune()
> to remove it from what-ever list it is now on to do the LRU
> accounting at that point.
>
> This makes it -really hard- to change the LRU implementation. The
> use of the per-sb LRU lock serialises movement of the dentries
> between the different lists and the removal of them, and this is the
> only reason that it works. If we want to break up the dentry LRU
> lock and lists into, say, per-node lists, we remove the only
> serialisation that allows this lru list/dispose list abuse to work.
>
> To make this work effectively, the dispose list has to be isolated
> from the LRU list - dentries have to be removed from the LRU
> *before* being placed on the dispose list. This means that the LRU
> accounting and isolation is completed before disposal is started,
> and that means we can change the LRU implementation freely in
> future.
>
> This means that dentries *must* be marked with DCACHE_SHRINK_LIST
> when they are placed on the dispose list so that we don't think that
> parent dentries found in try_prune_one_dentry() are on the LRU when
> the are actually on the dispose list. This would result in
> accounting the dentry to the LRU a second time. Hence
> dentry_lru_del() has to handle the DCACHE_SHRINK_LIST case
> differently because the dentry isn't on the LRU list.
>
> [ 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()
<sigh>
I need find a dealer that sells better crack.
> @@ -883,9 +923,16 @@ void shrink_dcache_sb(struct super_block *sb)
>
> spin_lock(&sb->s_dentry_lru_lock);
> while (!list_empty(&sb->s_dentry_lru)) {
> - list_splice_init(&sb->s_dentry_lru, &tmp);
> + /*
> + * account for removal here so we don't need to handle it later
> + * even though the dentry is no longer on the lru list.
> + */
> spin_unlock(&sb->s_dentry_lru_lock);
> - shrink_dentry_list(&tmp);
> + list_splice_init(&sb->s_dentry_lru, &tmp);
> + this_cpu_sub(nr_dentry_unused, sb->s_nr_dentry_unused);
> + sb->s_nr_dentry_unused = 0;
> +
> + shrink_dcache_list(&tmp);
> spin_lock(&sb->s_dentry_lru_lock);
This is now completely wrong. It should end up like this:
while (!list_empty(&sb->s_dentry_lru)) {
/*
* account for removal here so we don't need to handle it later
* even though the dentry is no longer on the lru list.
*/
list_splice_init(&sb->s_dentry_lru, &tmp);
this_cpu_sub(nr_dentry_unused, sb->s_nr_dentry_unused);
sb->s_nr_dentry_unused = 0;
spin_unlock(&sb->s_dentry_lru_lock);
shrink_dcache_list(&tmp);
spin_lock(&sb->s_dentry_lru_lock);
}
spin_unlock(&sb->s_dentry_lru_lock);
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>