linux-kernel - Re: [linux-next:master] [mm] 0fa2857d23: WARNING:at_mm/page_alloc.c:#__alloc_pages

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]

Message-ID: <wfsev2j7ehjsrwxacrbnokdfaozfgz7nnl56g4plcqhxgtglq3@3rwa3jbliafa>
Date: Mon, 24 Jun 2024 14:02:02 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Matthew Wilcox <willy@...radead.org>
Cc: Yosry Ahmed <yosryahmed@...gle.com>, 
	kernel test robot <oliver.sang@...el.com>, Usama Arif <usamaarif642@...il.com>, oe-lkp@...ts.linux.dev, 
	lkp@...el.com, Linux Memory Management List <linux-mm@...ck.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, Chengming Zhou <chengming.zhou@...ux.dev>, 
	Nhat Pham <nphamcs@...il.com>, David Hildenbrand <david@...hat.com>, 
	"Huang, Ying" <ying.huang@...el.com>, Hugh Dickins <hughd@...gle.com>, 
	Johannes Weiner <hannes@...xchg.org>, Andi Kleen <ak@...ux.intel.com>, linux-kernel@...r.kernel.org
Subject: Re: [linux-next:master] [mm] 0fa2857d23:
 WARNING:at_mm/page_alloc.c:#__alloc_pages_noprof

On Mon, Jun 24, 2024 at 09:51:33PM GMT, Matthew Wilcox wrote:
> On Mon, Jun 24, 2024 at 01:39:45PM -0700, Shakeel Butt wrote:
> > On Mon, Jun 24, 2024 at 08:50:45PM GMT, Matthew Wilcox wrote:
> > > On Mon, Jun 24, 2024 at 12:34:04PM -0700, Yosry Ahmed wrote:
> > > > On Mon, Jun 24, 2024 at 12:26 PM Matthew Wilcox <willy@...radead.org> wrote:
> > > > >
> > > > > On Mon, Jun 24, 2024 at 11:57:45AM -0700, Yosry Ahmed wrote:
> > > > > > On Mon, Jun 24, 2024 at 11:56 AM Matthew Wilcox <willy@...radead.org> wrote:
> > > > > > >
> > > > > > > On Mon, Jun 24, 2024 at 11:53:30AM -0700, Yosry Ahmed wrote:
> > > > > > > > After a page is swapped out during reclaim, __remove_mapping() will
> > > > > > > > call __delete_from_swap_cache() to replace the swap cache entry with a
> > > > > > > > shadow entry (which is an xa_value).
> > > > > > >
> > > > > > > Special entries are disjoint from shadow entries.  Shadow entries have
> > > > > > > the last two bits as 01 or 11 (are congruent to 1 or 3 modulo 4).
> > > > > > > Special entries have values below 4096 which end in 10 (are congruent
> > > > > > > to 2 modulo 4).
> > > > > >
> > > > > > You are implying that we would no longer have a shadow entry for such
> > > > > > zero folios, because we will be storing a special entry instead.
> > > > > > Right?
> > > > >
> > > > > umm ... maybe I have a misunderstanding here.
> > > > >
> > > > > I'm saying that there wouldn't be a _swap_ entry here because the folio
> > > > > wouldn't be stored anywhere on the swap device.  But there could be a
> > > > > _shadow_ entry.  Although if the page is full of zeroes, it was probably
> > > > > never referenced and doesn't really need a shadow entry.
> > > > 
> > > > Is it possible to have a shadow entry AND a special entry (e.g.
> > > > XA_ZERO_ENTRY) at the same index? This is what would be required to
> > > > maintain the current behavior (assuming we really need the shadow
> > > > entries for such zeroed folios).
> > > 
> > > No, just like it's not possible to have a swap entry and a shadow entry
> > > at the same location.  You have to choose.  But the zero entry is an
> > > alternative to the swap entry, not the shadow entry.
> > > 
> > > As I understand the swap cache, at the moment, you can have four
> > > possible results from a lookup:
> > > 
> > >  - NULL
> > >  - a swap entry
> > >  - a shadow entry
> > >  - a folio
> > > 
> > > Do I have that wrong?
> > 
> > I don't think we have swap entry in the swapcache (underlying xarray).
> > The swap entry is used as an index to find the folio or shadow entry.
> 
> Ah.  I think I understand the procedure now.
> 
> We store a swap entry in the page table entry.  That tells us both where
> in the swap cache the folio might be found, and where in the swap device
> the data can be found (because there is a very simple calculation for
> both).  If the folio is not present, then there's a shadow entry which
> summarises the LRU information that would be stored in the folio had it
> not been evicted from the swapcache.
> 
> We can't know at the point where we unmap the page whether it's full
> of zeroes or not, because we can't afford to scan its contents.  At the
> point where we decide to swap out the folio, we can afford to make that
> decision because the cost of doing the I/O is high enough.
> 
> So the question is whether we can afford to throw away the shadow
> information and just store the information that this was a zero entry.
> I think we can, but it is a more bold proposal than I realised I was
> making.

I agree that we can throw away shadow in the favor of zero entry but, as
you already noted, it requires changes at mutiple places. At the moment
I can think of:

1. Zero entry is not reclaimable like shadow entry.
2. Need to decide the right place to allocate the zero folio on swapin.
3. Should this be treated as major fault for stats purpose.

Definitely I have missed more points as well.