On Mon, Nov 18, 2019 at 08:23:12PM +0800, Alex Shi wrote:
> 在 2019/11/16 下午3:03, Shakeel Butt 写道:
> >> +reget_lruvec:
> >> + lruvec = mem_cgroup_page_lruvec(page, pgdat);
> >> +
> >> /* If we already hold the lock, we can skip some rechecking */
> >> - if (!locked) {
> >> - locked = compact_lock_irqsave(&pgdat->lru_lock,
> >> - &flags, cc);
> >> + if (lruvec != locked_lruvec) {
> >> + if (locked_lruvec) {
> >> + spin_unlock_irqrestore(&locked_lruvec->lru_lock,
> >> + locked_lruvec->irqflags);
> >> + locked_lruvec = NULL;
> >> + }
> > What guarantees the lifetime of lruvec? You should read the comment on
> > mem_cgroup_page_lruvec(). Have you seen the patches Hugh had shared?
> > Please look at the trylock_page_lruvec().
> >
>
> Thanks for comments, Shakeel.
>
> lruvec lifetime is same as memcg, which allocted in mem_cgroup_alloc()->alloc_mem_cgroup_per_node_info()
> I have read Hugh's patchset, even not every lines. But what's point of you here?
I believe Shakeel's point is that here:
struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgdat)
{
...
memcg = page->mem_cgroup;
there is nothing pinning the memcg, and it could be freed before
dereferencing memcg->nodeinfo in mem_cgroup_page_nodeinfo().
