linux-kernel - Re: [PATCH v2 2/4] mm/hugetlb: Refactor unmap_hugepage

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: <aAhbT2nzOyZ9b3ir@casper.infradead.org>
Date: Wed, 23 Apr 2025 04:15:27 +0100
From: Matthew Wilcox <willy@...radead.org>
To: nifan.cxl@...il.com
Cc: muchun.song@...ux.dev, mcgrof@...nel.org, a.manzanares@...sung.com,
	dave@...olabs.net, akpm@...ux-foundation.org, david@...hat.com,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Fan Ni <fan.ni@...sung.com>,
	Sidhartha Kumar <sidhartha.kumar@...cle.com>
Subject: Re: [PATCH v2 2/4] mm/hugetlb: Refactor unmap_hugepage_range() to
 take folio instead of page

On Fri, Apr 18, 2025 at 09:57:40AM -0700, nifan.cxl@...il.com wrote:
>  void unmap_hugepage_range(struct vm_area_struct *,
> -			  unsigned long, unsigned long, struct page *,
> +			  unsigned long, unsigned long, struct folio *folio,

I'm fine with leaving the vma and folio unnamed, but it is a crime
against our fellow programmers to leave the two 'unsigned long's
unnamed.  What the hell are they?

>  void unmap_hugepage_range(struct vm_area_struct *vma, unsigned long start,
> -			  unsigned long end, struct page *ref_page,
> +			  unsigned long end, struct folio *ref_folio,

... start and end.  I'd happily see a patch which only named those
parameters and left the struct folio unnamed.

> -	__unmap_hugepage_range(&tlb, vma, start, end, ref_page, zap_flags);
> +	__unmap_hugepage_range(&tlb, vma, start, end,
> +			       folio_page(ref_folio, 0), zap_flags);

I do not like this.  Why should we pass in the first page here?  It
seems to me that this is just "Now we will call a function which still
takes a struct page", and we *SHOULD* use &folio->page here to indicate
that we just haven't done the conversion yet.