linux-kernel - Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to huge_ptep

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: <ZlTFF7L3bcfUaJbg@localhost.localdomain>
Date: Mon, 27 May 2024 19:38:31 +0200
From: Oscar Salvador <osalvador@...e.de>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Jason Gunthorpe <jgg@...dia.com>, Peter Xu <peterx@...hat.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Nicholas Piggin <npiggin@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to
 huge_ptep_get()

On Mon, May 27, 2024 at 03:51:41PM +0000, Christophe Leroy wrote:
> We could be is that worth the churn ?

Probably not.

> With patch 1 there was only one callsite.

Yes, you are right here.

> Here we have many callsites, and we also have huge_ptep_get_and_clear() 
> which already takes three arguments. So for me it make more sense to 
> adapt huge_ptep_get() here.
> 
> Today several of the huge-related functions already have parameters that 
> are used only by a few architectures and everytime one architecture 
> needs a new parameter it is added for all of them, and there are 
> exemples in the past of new functions added to get new parameters for 
> only a few architectures that ended up with a mess and a need to 
> re-factor at the end.
> 
> See for instance the story around arch_make_huge_pte() and pte_mkhuge(), 
> both do the same but arch_make_huge_pte() was added to take additional 
> parameters by commit d9ed9faac283 ("mm: add new arch_make_huge_pte() 
> method for tile support") then they were merged by commit 16785bd77431 
> ("mm: merge pte_mkhuge() call into arch_make_huge_pte()")
> 
> So I'm open to any suggestion but we need to try not make it a bigger 
> mess at the end.
> 
> By the way, I think most if not all huge related helpers should all take 
> the same parameters even if not all of them are used, then it would make 
> things easier. And maybe the cleanest would be to give the page size to 
> all those functions instead of having them guess it.
> 
> So let's have your ideas here on the most straight forward way to handle 
> that.

It is probably not worth pursuing this then.
As you said, there are many callers and we would have to create some kind of hook
for only those interested places, which I guess would end up looking just too ugly
in order to save little code in arch code.

So please disregard my comment here, and stick with what we have.

> By the way, after commit 01d89b93e176 ("mm/gup: fix hugepd handling in 
> hugetlb rework") we now have the vma in gup_hugepte() so we now pass 
> vma->vm_mm

I did not notice, thanks.


-- 
Oscar Salvador
SUSE Labs