On Wed, 22 Nov 2017 06:36:45 +0100
Mike Galbraith <[email protected]> wrote:
> On Tue, 2017-11-21 at 22:50 -0500, Steven Rostedt wrote:
> >
> > Does it work if you revert the patch?
>
> That would restore the gripe. How about this..
Would it?
The gripe you report is:
refill_stock()
get_cpu_var()
drain_stock()
res_counter_uncharge()
res_counter_uncharge_until()
spin_lock() <== boom
But commit 3e32cb2e0a1 ("mm: memcontrol: lockless page counters")
changed that code to this:
static void drain_stock(struct memcg_stock_pcp *stock)
{
struct mem_cgroup *old = stock->cached;
if (stock->nr_pages) {
- unsigned long bytes = stock->nr_pages * PAGE_SIZE;
-
- res_counter_uncharge(&old->res, bytes);
+ page_counter_uncharge(&old->memory, stock->nr_pages);
if (do_swap_account)
- res_counter_uncharge(&old->memsw, bytes);
+ page_counter_uncharge(&old->memsw, stock->nr_pages);
stock->nr_pages = 0;
}
Where we replaced res_counter_uncharge() which is this:
u64 res_counter_uncharge_until(struct res_counter *counter,
struct res_counter *top,
unsigned long val)
{
unsigned long flags;
struct res_counter *c;
u64 ret = 0;
local_irq_save(flags);
for (c = counter; c != top; c = c->parent) {
u64 r;
spin_lock(&c->lock);
r = res_counter_uncharge_locked(c, val);
if (c == counter)
ret = r;
spin_unlock(&c->lock);
}
local_irq_restore(flags);
return ret;
}
u64 res_counter_uncharge(struct res_counter *counter, unsigned long val)
{
return res_counter_uncharge_until(counter, NULL, val);
}
and has that spin lock, to this:
void page_counter_cancel(struct page_counter *counter, unsigned long nr_pages)
{
long new;
new = atomic_long_sub_return(nr_pages, &counter->count);
/* More uncharges than charges? */
WARN_ON_ONCE(new < 0);
}
void page_counter_uncharge(struct page_counter *counter, unsigned long nr_pages)
{
struct page_counter *c;
for (c = counter; c; c = c->parent)
page_counter_cancel(c, nr_pages);
}
You see. No more spin lock to gripe about. No boom in your scenario.
-- Steve
