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
