[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7a9b79b-f1ab-4690-a3cf-4e9238e31790@huawei.com>
Date: Wed, 24 Jul 2024 20:54:57 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Alexander H Duyck <alexander.duyck@...il.com>, <davem@...emloft.net>,
<kuba@...nel.org>, <pabeni@...hat.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Andrew Morton
<akpm@...ux-foundation.org>, <linux-mm@...ck.org>
Subject: Re: [RFC v11 09/14] mm: page_frag: use __alloc_pages() to replace
alloc_pages_node()
On 2024/7/22 5:41, Alexander H Duyck wrote:
...
>> if (unlikely(!page)) {
>> - page = alloc_pages_node(NUMA_NO_NODE, gfp, 0);
>> + page = __alloc_pages(gfp, 0, numa_mem_id(), NULL);
>> if (unlikely(!page)) {
>> memset(nc, 0, sizeof(*nc));
>> return NULL;
>
> So if I am understanding correctly this is basically just stripping the
> checks that were being performed since they aren't really needed to
> verify the output of numa_mem_id.
>
> Rather than changing the code here, it might make more sense to update
> alloc_pages_node_noprof to move the lines from
> __alloc_pages_node_noprof into it. Then you could put the VM_BUG_ON and
> warn_if_node_offline into an else statement which would cause them to
> be automatically stripped for this and all other callers. The benefit
I suppose you meant something like below:
@@ -290,10 +290,14 @@ struct folio *__folio_alloc_node_noprof(gfp_t gfp, unsigned int order, int nid)
static inline struct page *alloc_pages_node_noprof(int nid, gfp_t gfp_mask,
unsigned int order)
{
- if (nid == NUMA_NO_NODE)
+ if (nid == NUMA_NO_NODE) {
nid = numa_mem_id();
+ } else {
+ VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES);
+ warn_if_node_offline(nid, gfp_mask);
+ }
- return __alloc_pages_node_noprof(nid, gfp_mask, order);
+ return __alloc_pages_noprof(gfp_mask, order, nid, NULL);
}
> would likely be much more significant and may be worthy of being
> accepted on its own merit without being a part of this patch set as I
> would imagine it would show slight gains in terms of performance and
> binary size by dropping the unnecessary instructions.
Below is the result, it does reduce the binary size for
__page_frag_alloc_align() significantly as expected, but also
increase the size for other functions, which seems to be passing
a runtime nid, so the trick above doesn't work. I am not sure if
the overall reduction is significant enough to justify the change?
It seems that depends on how many future callers are passing runtime
nid to alloc_pages_node() related APIs.
[linyunsheng@...alhost net-next]$ ./scripts/bloat-o-meter vmlinux.org vmlinux
add/remove: 1/2 grow/shrink: 13/8 up/down: 160/-256 (-96)
Function old new delta
bpf_map_alloc_pages 708 764 +56
its_probe_one 2836 2860 +24
iommu_dma_alloc 984 1008 +24
__iommu_dma_alloc_noncontiguous.constprop 1180 1192 +12
e843419@...f_00011fb1_4348 - 8 +8
its_vpe_irq_domain_deactivate 312 316 +4
its_vpe_irq_domain_alloc 1492 1496 +4
its_irq_domain_free 440 444 +4
iommu_dma_map_sg 1328 1332 +4
dpaa_eth_probe 5524 5528 +4
dpaa2_eth_xdp_xmit 676 680 +4
dpaa2_eth_open 564 568 +4
dma_direct_get_required_mask 116 120 +4
__dma_direct_alloc_pages.constprop 656 660 +4
its_vpe_set_affinity 928 924 -4
its_send_single_command 340 336 -4
its_alloc_table_entry 456 452 -4
dpaa_bp_seed 232 228 -4
arm_64_lpae_alloc_pgtable_s1 680 676 -4
__arm_lpae_alloc_pages 900 896 -4
e843419@...3_00005079_16ec 8 - -8
e843419@...9_00001c33_1c8 8 - -8
ringbuf_map_alloc 612 600 -12
__page_frag_alloc_align 740 536 -204
Total: Before=30306836, After=30306740, chg -0.00%
Powered by blists - more mailing lists