Received-SPF: pass (google.com: domain of linux-kernel+bounces-191297-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Mon, 27 May 2024 19:38:31 +0200
From: Oscar Salvador <osalvador@suse.de>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jason Gunthorpe <jgg@nvidia.com>, Peter Xu <peterx@redhat.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [RFC PATCH v3 03/16] mm: Provide mm_struct and address to
 huge_ptep_get()
Message-ID: <ZlTFF7L3bcfUaJbg@localhost.localdomain>
References: <cover.1716714720.git.christophe.leroy@csgroup.eu>
 <fbba60d762faad40ebb959bf9517c5c22301f69e.1716714720.git.christophe.leroy@csgroup.eu>
 <ZlRsMCvVo9tSEFQV@localhost.localdomain>
 <22c4ba7c-28d2-43bd-81b6-bd63f77d1d9e@csgroup.eu>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <22c4ba7c-28d2-43bd-81b6-bd63f77d1d9e@csgroup.eu>

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