lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191211194933.15b53c11@carbon> Date: Wed, 11 Dec 2019 19:49:33 +0100 From: Jesper Dangaard Brouer <brouer@...hat.com> To: Saeed Mahameed <saeedm@...lanox.com> Cc: "jonathan.lemon@...il.com" <jonathan.lemon@...il.com>, "linyunsheng@...wei.com" <linyunsheng@...wei.com>, Li Rongqing <lirongqing@...du.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "ilias.apalodimas@...aro.org" <ilias.apalodimas@...aro.org>, brouer@...hat.com Subject: Re: [PATCH][v2] page_pool: handle page recycle for NUMA_NO_NODE condition On Sat, 7 Dec 2019 03:52:41 +0000 Saeed Mahameed <saeedm@...lanox.com> wrote: > 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 agree, using numa_mem_id() is not valid, because it takes the numa node id from the executing CPU and the call to __page_pool_put_page() can happen on a remote CPU (e.g. cpumap redirect, and in future SKBs). > I suggest the following: > > return !page_pfmemalloc() && > ( page_to_nid(page) == pool->p.nid || pool->p.nid == NUMA_NO_NODE ); Above code doesn't generate optimal ASM code, I suggest: static bool pool_page_reusable(struct page_pool *pool, struct page *page) { return !page_is_pfmemalloc(page) && pool->p.nid != NUMA_NO_NODE && page_to_nid(page) == pool->p.nid; } I have compiled different variants and looked at the ASM code generated by GCC. This seems to give the best result. > 1) never recycle emergency pages, regardless of pool nid. > 2) always recycle if pool is NUMA_NO_NODE. Yes, this defines the semantics, that a page_pool configured with NUMA_NO_NODE means skip NUMA checks. I think that sounds okay... > the above change should not add any overhead, a modest branch > predictor will handle this with no effort. It still annoys me that we keep adding instructions to this code hot-path (I counted 34 bytes and 11 instructions in my proposed function). I think that it might be possible to move these NUMA checks to alloc-side (instead of return/recycles side as today), and perhaps only on slow-path when dequeuing from ptr_ring (as recycles that call __page_pool_recycle_direct() will be pinned during NAPI). But lets focus on a smaller fix for the immediate issue... -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Powered by blists - more mailing lists