| Message ID | 20250114021922.92609-5-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf, mm: Introduce try_alloc_pages() | expand |
On Mon 13-01-25 18:19:20, Alexei Starovoitov wrote: > From: Alexei Starovoitov <ast@kernel.org> > > Teach memcg to operate under trylock conditions when > spinning locks cannot be used. > The end result is __memcg_kmem_charge_page() and > __memcg_kmem_uncharge_page() are safe to use from > any context in RT and !RT. > In !RT the NMI handler may fail to trylock stock_lock. > In RT hard IRQ and NMI handlers will not attempt to trylock. I believe this is local_trylock_irqsave specific thing that is not that interesting for the particular code path. It is more useful to mention consequences. I would phrase it this way. local_trylock might fail and this would lead to charge cache bypass if the calling context doesn't allow spinning (gfpflags_allow_spinning). In those cases we try to charge the memcg counter directly and fail early if that is not possible. This might cause a pre-mature charge failing but it will allow an opportunistic charging that is safe from try_alloc_pages path. > Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/memcontrol.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 7b3503d12aaf..e4c7049465e0 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1756,7 +1756,8 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, > * > * returns true if successful, false otherwise. > */ > -static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > +static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages, > + gfp_t gfp_mask) > { > struct memcg_stock_pcp *stock; > unsigned int stock_pages; > @@ -1766,7 +1767,11 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > if (nr_pages > MEMCG_CHARGE_BATCH) > return ret; > > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { > + if (!gfpflags_allow_spinning(gfp_mask)) > + return ret; > + local_lock_irqsave(&memcg_stock.stock_lock, flags); > + } > > stock = this_cpu_ptr(&memcg_stock); > stock_pages = READ_ONCE(stock->nr_pages); > @@ -1851,7 +1856,14 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > unsigned long flags; > > - local_lock_irqsave(&memcg_stock.stock_lock, flags); > + if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { > + /* > + * In case of unlikely failure to lock percpu stock_lock > + * uncharge memcg directly. > + */ > + mem_cgroup_cancel_charge(memcg, nr_pages); > + return; > + } > __refill_stock(memcg, nr_pages); > local_unlock_irqrestore(&memcg_stock.stock_lock, flags); > } > @@ -2196,9 +2208,13 @@ int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, > unsigned long pflags; > > retry: > - if (consume_stock(memcg, nr_pages)) > + if (consume_stock(memcg, nr_pages, gfp_mask)) > return 0; > > + if (!gfpflags_allow_spinning(gfp_mask)) > + /* Avoid the refill and flush of the older stock */ > + batch = nr_pages; > + > if (!do_memsw_account() || > page_counter_try_charge(&memcg->memsw, batch, &counter)) { > if (page_counter_try_charge(&memcg->memory, batch, &counter)) > -- > 2.43.5
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 7b3503d12aaf..e4c7049465e0 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1756,7 +1756,8 @@ static bool obj_stock_flush_required(struct memcg_stock_pcp *stock, * * returns true if successful, false otherwise. */ -static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) +static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages, + gfp_t gfp_mask) { struct memcg_stock_pcp *stock; unsigned int stock_pages; @@ -1766,7 +1767,11 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) if (nr_pages > MEMCG_CHARGE_BATCH) return ret; - local_lock_irqsave(&memcg_stock.stock_lock, flags); + if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { + if (!gfpflags_allow_spinning(gfp_mask)) + return ret; + local_lock_irqsave(&memcg_stock.stock_lock, flags); + } stock = this_cpu_ptr(&memcg_stock); stock_pages = READ_ONCE(stock->nr_pages); @@ -1851,7 +1856,14 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) { unsigned long flags; - local_lock_irqsave(&memcg_stock.stock_lock, flags); + if (!local_trylock_irqsave(&memcg_stock.stock_lock, flags)) { + /* + * In case of unlikely failure to lock percpu stock_lock + * uncharge memcg directly. + */ + mem_cgroup_cancel_charge(memcg, nr_pages); + return; + } __refill_stock(memcg, nr_pages); local_unlock_irqrestore(&memcg_stock.stock_lock, flags); } @@ -2196,9 +2208,13 @@ int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask, unsigned long pflags; retry: - if (consume_stock(memcg, nr_pages)) + if (consume_stock(memcg, nr_pages, gfp_mask)) return 0; + if (!gfpflags_allow_spinning(gfp_mask)) + /* Avoid the refill and flush of the older stock */ + batch = nr_pages; + if (!do_memsw_account() || page_counter_try_charge(&memcg->memsw, batch, &counter)) { if (page_counter_try_charge(&memcg->memory, batch, &counter))