Re: [PATCH v10 11/35] list_lru: per-node list infrastructure

Glauber Costa
2013-06-06T16:15:07Z

On 06/06/2013 03:08 AM, Andrew Morton wrote:
>> + for_each_node_mask(nid, lru->active_nodes) {
>> > + struct list_lru_node *nlru = &lru->node[nid];
>> > +
>> > + spin_lock(&nlru->lock);
>> > + BUG_ON(nlru->nr_items < 0);
> This is buggy.
>
> The bit in lru->active_nodes could be cleared by now. We can only make
> this assertion if we recheck lru->active_nodes[nid] inside the
> spinlocked region.
>
Sorry Andrew, how so ?
We will clear that flag if nr_items == 0. nr_items should *never* get to
be less than 0, it doesn't matter if the node is cleared or not.

If the node is cleared, we would expected the following statement to
expand to
count += nlru->nr_items = 0;
spin_unlock(&nlru->lock);

Which is actually cheaper than testing for the bit being still set.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: email@kvack.org