linux-kernel - Re: [PATCH v3 2/6] mm: remove swap_free() and always use swap_free

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] [thread-next>] [day] [month] [year] [list]

Date: Fri, 3 May 2024 13:37:06 -0700
From: Chris Li <chrisl@...nel.org>
To: Ryan Roberts <ryan.roberts@....com>
Cc: Barry Song <21cnbao@...il.com>, akpm@...ux-foundation.org, linux-mm@...ck.org, 
	baolin.wang@...ux.alibaba.com, david@...hat.com, hanchuanhua@...o.com, 
	hannes@...xchg.org, hughd@...gle.com, kasong@...cent.com, 
	linux-kernel@...r.kernel.org, surenb@...gle.com, v-songbaohua@...o.com, 
	willy@...radead.org, xiang@...nel.org, ying.huang@...el.com, 
	yosryahmed@...gle.com, yuzhao@...gle.com, ziy@...dia.com, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>
Subject: Re: [PATCH v3 2/6] mm: remove swap_free() and always use swap_free_nr()

On Fri, May 3, 2024 at 2:31 AM Ryan Roberts <ryan.roberts@....com> wrote:
>
> On 03/05/2024 01:50, Barry Song wrote:
> > From: Barry Song <v-songbaohua@...o.com>
> >
> > To streamline maintenance efforts, we propose discontinuing the use of
> > swap_free(). Instead, we can simply invoke swap_free_nr() with nr set
> > to 1. This adjustment offers the advantage of enabling batch processing
> > within kernel/power/swap.c. Furthermore, swap_free_nr() is designed with
> > a bitmap consisting of only one long, resulting in overhead that can be
> > ignored for cases where nr equals 1.
> >
> > Suggested-by: "Huang, Ying" <ying.huang@...el.com>
> > Signed-off-by: Barry Song <v-songbaohua@...o.com>
> > Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> > Cc: Pavel Machek <pavel@....cz>
> > Cc: Len Brown <len.brown@...el.com>
> > Cc: Hugh Dickins <hughd@...gle.com>
> > ---
> >  include/linux/swap.h |  5 -----
> >  kernel/power/swap.c  |  7 +++----
> >  mm/memory.c          |  2 +-
> >  mm/rmap.c            |  4 ++--
> >  mm/shmem.c           |  4 ++--
> >  mm/swapfile.c        | 19 +++++--------------
> >  6 files changed, 13 insertions(+), 28 deletions(-)
> >
> > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > index d1d35e92d7e9..f03cb446124e 100644
> > --- a/include/linux/swap.h
> > +++ b/include/linux/swap.h
> > @@ -482,7 +482,6 @@ extern int add_swap_count_continuation(swp_entry_t, gfp_t);
> >  extern void swap_shmem_alloc(swp_entry_t);
> >  extern int swap_duplicate(swp_entry_t);
> >  extern int swapcache_prepare(swp_entry_t);
> > -extern void swap_free(swp_entry_t);
>
> I wonder if it would be cleaner to:
>
> #define swap_free(entry) swap_free_nr((entry), 1)
>
> To save all the churn for the callsites that just want to pass a single entry?
>
Either way works. It will produce the same machine code. I have a
slight inclination to just drop swap_free(entry) API so that it
discourages the caller to do a for loop over swap_free().

Acked-by: Chris Li <chrisl@...nel.org>

Chris