Re: list_lru isolate callback question?

[Dave Chinner <david@fromorbit.com>]

On Thu, Jun 05, 2025 at 07:22:23PM +1000, Dave Airlie wrote:
> On Thu, 5 Jun 2025 at 17:55, Kairui Song <ryncsn@gmail.com> wrote:
> >
> > On Thu, Jun 5, 2025 at 10:17 AM Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > I've hit a case where I think it might be valuable to have the nid +
> > > struct memcg for the item being iterated available in the isolate
> > > callback, I know in theory we should be able to retrieve it from the
> > > item, but I'm also not convinced we should need to since we have it
> > > already in the outer function?
> > >
> > > typedef enum lru_status (*list_lru_walk_cb)(struct list_head *item,
> > >                         struct list_lru_one *list,
> > >                         int nid,
> > >                         struct mem_cgroup *memcg,
> > >                         void *cb_arg);
> > >
> >
> > Hi Dave,
> >
> > > It's probably not essential (I think I can get the nid back easily,
> > > not sure about the memcg yet), but I thought I'd ask if there would be
> >
> > If it's a slab object you should be able to get it easily with:
> > memcg = mem_cgroup_from_slab_obj(item));
> > nid = page_to_nid(virt_to_page(item));
> >
> 
> It's in relation to some work trying to tie GPU system memory
> allocations into memcg properly,
> 
> Not slab objects, but I do have pages so I'm using page_to_nid right now,
> however these pages aren't currently setting p->memcg_data as I don't
> need that for this, but maybe
> this gives me a reason to go down that road.

How are you accounting the page to the memcg if the page is not
marked as owned by as specific memcg?

Are you relying on the page being indexed in a specific list_lru to
account for the page correcting in reclaim contexts, and that's why
you need this information in the walk context?

I'd actually like to know more details of the problem you are trying
to solve - all I've heard is "we're trying to do <something> with
GPUs and memcgs with list_lrus", but I don't know what it is so I
can't really give decent feedback on your questions....

> > > resistance against just adding them to the callback?
> >
> > I'm not sure about the context here, I personally prefer to keep the
> > function minimized unless necessary, so things like !CONFIG_MEMCG or
> > single node builds won't have two dummy parameters here, and
> > most caller won't need them, the compiler can't optimize that
> > out IIUC.
> 
> I reconsidered and was wondering if
> 
> struct lru_walk_args {
>     struct list_lru_one *list;
>     int nid;
>     struct mem_cgroup *memcg;
> }
> would also be an option instead of adding two unused args.

The walk function is passed a struct list_lru_one. If there is a
need to get the {nid,memcg} of the objects efficiently from walk
contexts, then we should encode them into the struct list_lru_one
at init time and retreive them from there.

-Dave.
-- 
Dave Chinner
david@fromorbit.com