linux-kernel - Re: [PATCH] mm/page_alloc: call check_pcp_refill() while zone spinlock is not held

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 14 Mar 2022 10:00:46 +0000
From:   Mel Gorman <mgorman@...hsingularity.net>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Matthew Wilcox <willy@...radead.org>,
        Shakeel Butt <shakeelb@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Michal Hocko <mhocko@...nel.org>, Wei Xu <weixugc@...gle.com>,
        Greg Thelen <gthelen@...gle.com>,
        Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH] mm/page_alloc: call check_pcp_refill() while zone
 spinlock is not held

On Sun, Mar 13, 2022 at 04:25:47PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@...gle.com>
> 
> check_pcp_refill() is used from rmqueue_bulk() while zone spinlock
> is held.
> 
> This used to be fine because check_pcp_refill() was testing only the
> head page, while its 'struct page' was very hot in the cpu caches.
> 
> With ("mm/page_alloc: check high-order pages for corruption during PCP
> operations") check_pcp_refill() will add latencies for high order pages.
> 
> We can defer the calls to check_pcp_refill() after the zone
> spinlock has been released.
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>

I'm not a major fan. While this reduces the lock hold times, it adds
another list walk which may make the overall operation more expensive
which is probably a net loss given that the cold struct pages are still
accessed.

The lower lock hold times applies to high-order allocations which are
either THPs or SLAB refills. THP can be expensive anyway depending on
whether compaction had to be used and SLAB refills do not necessarily occur
for every SLAB allocation (although it is likely much more common for
network intensive workloads). This means that the patch may be helping
the uncommon case (high order alloc) at the cost of the common case
(order-0 alloc).

Because this incurs a second list walk cost to the common case, I think
the changelog would need justification that it does not hurt common paths
and that the lock hold reduction times make a meaningful difference.

-- 
Mel Gorman
SUSE Labs