linux-kernel - Re: [PATCH v7 05/11] mm: split a folio in minimum folio order chunks

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: <20240607204552.7bmjf36bsupeznkq@quentin>
Date: Fri, 7 Jun 2024 20:45:52 +0000
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Zi Yan <ziy@...dia.com>, david@...morbit.com, djwong@...nel.org,
	chandan.babu@...cle.com, brauner@...nel.org,
	akpm@...ux-foundation.org, mcgrof@...nel.org, linux-mm@...ck.org,
	hare@...e.de, linux-kernel@...r.kernel.org,
	yang@...amperecomputing.com, linux-xfs@...r.kernel.org,
	p.raghav@...sung.com, linux-fsdevel@...r.kernel.org, hch@....de,
	gost.dev@...sung.com, cl@...amperecomputing.com,
	john.g.garry@...cle.com, kirill.shutemov@...ux.intel.com
Subject: Re: [PATCH v7 05/11] mm: split a folio in minimum folio order chunks

On Fri, Jun 07, 2024 at 06:01:56PM +0100, Matthew Wilcox wrote:
> On Fri, Jun 07, 2024 at 12:58:33PM -0400, Zi Yan wrote:
> > > +int split_folio_to_list(struct folio *folio, struct list_head *list)
> > > +{
> > > +	unsigned int min_order = 0;
> > > +
> > > +	if (!folio_test_anon(folio)) {
> > > +		if (!folio->mapping) {
> > > +			count_vm_event(THP_SPLIT_PAGE_FAILED);
> > 
> > You should only increase this counter when the input folio is a THP, namely
> > folio_test_pmd_mappable(folio) is true. For other large folios, we will
> > need a separate counter. Something like MTHP_STAT_FILE_SPLIT_FAILED.
> > See enum mthp_stat_item in include/linux/huge_mm.h.
> 
> Also, why should this count as a split failure?  If we see a NULL
> mapping, the folio has been truncated and so no longer needs to be
> split.  I understand we currently count it as a failure, but I
> don't think we should.

I also thought about this. Because if the folio was under writeback, we
don't account it as a failure but we do it if it was truncated?

I can remove the accounting that we added as a part of this series in
the next version but address the upstream changes [1] in a separate
standalone patch?
I prefer to address these kind of open discussion upstream changes
separately so that we don't delay this series.

Let me know what you think. CCing Kirill as he made those changes.

[1]
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 399a4f5125c7..21f2dd5eb4c5 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3152,10 +3152,8 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
                mapping = folio->mapping;
 
                /* Truncated ? */
-               if (!mapping) {
+               if (!mapping)
                        ret = -EBUSY;
-                       goto out;
-               }