linux-kernel - Re: [PATCH v2 10/15] mm, swap: wrap swap cache replacement with a helper

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 for Android: free password hash cracker in your pocket

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

Message-ID: <9b4808cd-3239-4bb2-8073-7eb1412c4529@redhat.com>
Date: Mon, 8 Sep 2025 16:39:24 +0200
From: David Hildenbrand <david@...hat.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 Matthew Wilcox <willy@...radead.org>, Hugh Dickins <hughd@...gle.com>,
 Chris Li <chrisl@...nel.org>, Barry Song <baohua@...nel.org>,
 Baoquan He <bhe@...hat.com>, Nhat Pham <nphamcs@...il.com>,
 Kemeng Shi <shikemeng@...weicloud.com>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Ying Huang <ying.huang@...ux.alibaba.com>,
 Johannes Weiner <hannes@...xchg.org>, Yosry Ahmed <yosryahmed@...gle.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Zi Yan <ziy@...dia.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 10/15] mm, swap: wrap swap cache replacement with a
 helper

On 08.09.25 16:20, Kairui Song wrote:
> On Mon, Sep 8, 2025 at 8:35 PM David Hildenbrand <david@...hat.com> wrote:
>>
>>
>>>
>>> +/**
>>> + * __swap_cache_replace_folio - Replace a folio in the swap cache.
>>> + * @mapping: Swap mapping address space.
>>> + * @entry: The first swap entry that the new folio corresponds to.
>>> + * @old: The old folio to be replaced.
>>> + * @new: The new folio.
>>> + *
>>> + * Replace a existing folio in the swap cache with a new folio.
>>> + *
>>> + * Context: Caller must ensure both folios are locked, and lock the
>>> + * swap address_space that holds the entries to be replaced.
>>> + */
>>> +void __swap_cache_replace_folio(struct address_space *mapping,
>>> +                             swp_entry_t entry,
>>> +                             struct folio *old, struct folio *new)
>>
>> Can't we just use "new->swap.val" directly and avoid passing in the
>> entry, documenting that new->swap.val must be setup properly in advance?
> 
> Thanks for the suggestion.
> 
> I was thinking about the opposite. I think maybe it's better that the
> caller never sets the new folio's entry value, so folio->swap is always
> modified in mm/swap_state.c, and let __swap_cache_replace_folio set
> new->swap, to make it easier to track the folio->swap
> usage.
> 
> This can be done easily for migration and shmem parts, the huge split
> code will need a bit more cleanup.

Right, but it's probably worth it.

> 
> It's a trivial change I think. But letting __swap_cache_replace_folio
> setup new's swap and flags may deduplicate some code. So I thought
> maybe this can be better cleaned up later. So for now I just add a
> debug check here that `entry == new->swap`.
> 
> And the debug check does imply that we can just drop the entry params
> in this patch, there will be no feature change.

Well, the current API as you introduce it here is confusing, as it's not 
clear who is supposed to initialize what.

So better to it cleanly right from the start.

> 
>> Similarly, can't we obtain "mapping" from new?
> 
> This is doable. But this patch is only an intermediate patch, next
> commit will let the pass in ci instead. Of course the `ci` can be
> retrieved from `entry` directly too, but it's the caller's
> responsibility to lock the `ci`, so passing in a locked ci explicitly
> might be more intuitive? Also might save a tiny bit of CPU time from
> recalculating and load the `ci`.

Well, no other swap_cache_* functions consumes an address space, right?

-- 
Cheers

David / dhildenb