Re: [PATCH v2] mm: folio_zero_user: open code range computation in folio_zero_user()

[Ankur Arora]

David Hildenbrand (Arm) <david@xxxxxxxxxx> writes:

> On 2/5/26 06:48, Ankur Arora wrote:
>> Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> writes:
>>
>>> On Wed, 4 Feb 2026 22:01:42 +0100 "David Hildenbrand (arm)" <david@xxxxxxxxxx> wrote:
>>>
>>>>
>>>> I'm late, maybe this is already upstream.
>>>
>>> It's in mm-unstable.  The second round of MM upstreaming is two weeks hence.
>>>
>>>>
>>>> TBH, without the clamp that looks much more readable here.
>>>
>>> me too.
>>>
>>>>
>>>> Is that cast really required?
>>>
>>> Seems not.  The types for nr_pages are a bit chaotic - u64->long->uint.
>> Yes agreed.
>> The first u64 is because currently struct range only supports that.
>> Then the cast to signed long is because the range can be negative
>> and the clear_contig_highpages() is only done if nr_pages > 0.
>
> That makes sense to me.
>
>> And, the third one is almost certainly unnecessary for any realistic
>> hugepage size but since nr_pages is being truncating, I wanted that
>> to be explicit.
>
> But the non-silent truncation is no better? IOW, it doesn't matter.

I never seem to get them but I thought we had some kconfig option that
makes gcc give a warning to that effect.

I can update this patch to just implicitly truncate.

> You could just make clear_contig_highpages() consume an unsigned long ...

Unfortunately that'll be an even bigger mess. The clear_contig_highpages()
version in mm-stable uses the unsigned intness of nr_pages all over:

  static void clear_contig_highpages(struct page *page, unsigned long addr,
				   unsigned int nr_pages)
  {
	unsigned int i, count;
	/*
	 * When clearing we want to operate on the largest extent possible to
	 * allow for architecture specific extent based optimizations.
	 *
	 * However, since clear_user_highpages() (and primitives clear_user_pages(),
	 * clear_pages()), do not call cond_resched(), limit the unit size when
	 * running under non-preemptible scheduling models.
	 */
	const unsigned int unit = preempt_model_preemptible() ?
				   nr_pages : PROCESS_PAGES_NON_PREEMPT_BATCH;

	might_sleep();

	for (i = 0; i < nr_pages; i += count) {
		cond_resched();

		count = min(unit, nr_pages - i);
		clear_user_highpages(page + i, addr + i * PAGE_SIZE, count);
	}
  }

Thanks
--
ankur