list_lru: remove special case function list_lru_dispose_all.

The list_lru implementation has one function, list_lru_dispose_all, with
only one user (the dentry code).  At first, such function appears to make
sense because we are really not interested in the result of isolating each
dentry separately - all of them are going away anyway.  However, it's
implementation is buggy in the following way:

When we call list_lru_dispose_all in fs/dcache.c, we scan all dentries
marking them with DCACHE_SHRINK_LIST.  However, this is done without the
nlru->lock taken.  The imediate result of that is that someone else may
add or remove the dentry from the LRU at the same time.  When list_lru_del
happens in that scenario we will see an element that is not yet marked
with DCACHE_SHRINK_LIST (even though it will be in the future) and
obviously remove it from an lru where the element no longer is.  Since
list_lru_dispose_all will in effect count down nlru's nr_items and
list_lru_del will do the same, this will lead to an imbalance.

The solution for this would not be so simple: we can obviously just keep
the lru_lock taken, but then we have no guarantees that we will be able to
acquire the dentry lock (dentry->d_lock).  To properly solve this, we need
a communication mechanism between the lru and dentry code, so they can
coordinate this with each other.

Such mechanism already exists in the form of the list_lru_walk_cb
callback.  So it is possible to construct a dcache-side prune function
that does the right thing only by calling list_lru_walk in a loop until no
more dentries are available.

With only one user, plus the fact that a sane solution for the problem
would involve boucing between dcache and list_lru anyway, I see little
justification to keep the special case list_lru_dispose_all in tree.

Change-Id: I7cbc4646a323aae9605dac32e0a1591340493245
Signed-off-by: Glauber Costa <glommer@openvz.org>
Cc: Michal Hocko <mhocko@suse.cz>
Acked-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
This commit is contained in:
Glauber Costa 2013-08-28 10:18:03 +10:00 committed by followmsi
parent 87f6e97d3e
commit 9fda83a755
2 changed files with 0 additions and 59 deletions

View file

@ -137,21 +137,4 @@ list_lru_walk(struct list_lru *lru, list_lru_walk_cb isolate,
} }
return isolated; return isolated;
} }
typedef void (*list_lru_dispose_cb)(struct list_head *dispose_list);
/**
* list_lru_dispose_all: forceably flush all elements in an @lru
* @lru: the lru pointer
* @dispose: callback function to be called for each lru list.
*
* This function will forceably isolate all elements into the dispose list, and
* call the @dispose callback to flush the list. Please note that the callback
* should expect items in any state, clean or dirty, and be able to flush all of
* them.
*
* Return value: how many objects were freed. It should be equal to all objects
* in the list_lru.
*/
unsigned long
list_lru_dispose_all(struct list_lru *lru, list_lru_dispose_cb dispose);
#endif /* _LRU_LIST_H */ #endif /* _LRU_LIST_H */

View file

@ -112,48 +112,6 @@ restart:
} }
EXPORT_SYMBOL_GPL(list_lru_walk_node); EXPORT_SYMBOL_GPL(list_lru_walk_node);
static unsigned long list_lru_dispose_all_node(struct list_lru *lru, int nid,
list_lru_dispose_cb dispose)
{
struct list_lru_node *nlru = &lru->node[nid];
LIST_HEAD(dispose_list);
unsigned long disposed = 0;
spin_lock(&nlru->lock);
while (!list_empty(&nlru->list)) {
list_splice_init(&nlru->list, &dispose_list);
disposed += nlru->nr_items;
nlru->nr_items = 0;
node_clear(nid, lru->active_nodes);
spin_unlock(&nlru->lock);
dispose(&dispose_list);
spin_lock(&nlru->lock);
}
spin_unlock(&nlru->lock);
return disposed;
}
unsigned long list_lru_dispose_all(struct list_lru *lru,
list_lru_dispose_cb dispose)
{
unsigned long disposed;
unsigned long total = 0;
int nid;
do {
disposed = 0;
for_each_node_mask(nid, lru->active_nodes) {
disposed += list_lru_dispose_all_node(lru, nid,
dispose);
}
total += disposed;
} while (disposed != 0);
return total;
}
int list_lru_init(struct list_lru *lru) int list_lru_init(struct list_lru *lru)
{ {
int i; int i;