答复: 答复: [PATCH][v2] page_pool: handle page recycle for NUMA_NO_NODE condition

["Li,Rongqing" <lirongqing@baidu.com>]

> -----邮件原件-----
> 发件人: Ilias Apalodimas [mailto:ilias.apalodimas@linaro.org]
> 发送时间: 2019年12月9日 17:30
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: Yunsheng Lin <linyunsheng@huawei.com>; Saeed Mahameed
> <saeedm@mellanox.com>; jonathan.lemon@gmail.com;
> netdev@vger.kernel.org; brouer@redhat.com; ivan.khoronzhuk@linaro.org;
> grygorii.strashko@ti.com
> 主题: Re: 答复: [PATCH][v2] page_pool: handle page recycle for
> NUMA_NO_NODE condition
> 
> On Mon, Dec 09, 2019 at 03:47:50AM +0000, Li,Rongqing wrote:
> > > >
> [...]
> > > > Cc'ed Jesper, Ilias & Jonathan.
> > > >
> > > > I don't think it is correct to check that the page nid is same as
> > > > numa_mem_id() if pool is NUMA_NO_NODE. In such case we should
> > > > allow all pages to recycle, because you can't assume where pages
> > > > are allocated from and where they are being handled.
> > > >
> > > > I suggest the following:
> > > >
> > > > return !page_pfmemalloc() &&
> > > > ( page_to_nid(page) == pool->p.nid || pool->p.nid == NUMA_NO_NODE
> > > > );
> > > >
> > > > 1) never recycle emergency pages, regardless of pool nid.
> > > > 2) always recycle if pool is NUMA_NO_NODE.
> > >
> > > As I can see, below are the cases that the pool->p.nid could be
> > > NUMA_NO_NODE:
> > >
> > > 1. kernel built with the CONFIG_NUMA being off.
> > >
> > > 2. kernel built with the CONFIG_NUMA being on, but FW/BIOS dose not
> provide
> > >    a valid node id through ACPI/DT, and it has the below cases:
> > >
> > >    a). the hardware itself is single numa node system, so maybe it is valid
> > >        to not provide a valid node for the device.
> > >    b). the hardware itself is multi numa nodes system, and the FW/BIOS is
> > >        broken that it does not provide a valid one.
> > >
> > > 3. kernel built with the CONFIG_NUMA being on, and FW/BIOS dose provide
> a
> > >    valid node id through ACPI/DT, but the driver does not pass the valid
> > >    node id when calling page_pool_init().
> > >
> > > I am not sure which case this patch is trying to fix, maybe Rongqing
> > > can help to clarify.
> > >
> > > For case 1 and case 2 (a), I suppose checking pool->p.nid being
> > > NUMA_NO_NODE is enough.
> > >
> > > For case 2 (b), There may be argument that it should be fixed in the
> > > BIOS/FW, But it also can be argued that the numa_mem_id() checking
> > > has been done in the driver that has not using page pool yet when
> > > deciding whether to do recycling, see [1]. If I understanding
> > > correctly, recycling is normally done at the NAPI pooling, which has
> > > the same affinity as the rx interrupt, and rx interrupt is not
> > > changed very often. If it does change to different memory node, maybe it
> makes sense not to recycle the old page belongs to old node?
> > >
> > >
> > > For case 3, I am not sure if any driver is doing that, and if the
> > > page pool API even allow that?
> > >
> >
> > I think pool_page_reusable should support NUMA_NO_NODE no matter
> which
> > cases
> >
> 
> Yes
> 
> >
> > And recycling is normally done at the NAPI pooling, NUMA_NO_NODE hint
> > to use the local node, except memory usage is unbalance, so I add the
> > check that the page nid is same as numa_mem_id() if pool is
> > NUMA_NO_NODE
> 
> I am not sure i am following here.
> Recycling done at NAPI or not should have nothing to do with NUMA.
> What do you mean by 'memory usage is unbalance'?
> 
> 

I am saying the possible reason that alloc_pages_node(NUMA_NO_NODE) can not
allocate local page, which causes the pool_page_reusable always return false if add the
check that the page nid is same as numa_mem_id()

it should rarely happen

-Li
> Thanks
> /Ilias