>>
>> zap_page_range_single_batched(
>> diff --git a/mm/memory.c b/mm/memory.c
>> index fdcd2abf29c2..7d7c24c6917c 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -1554,11 +1554,13 @@ copy_page_range(struct vm_area_struct *dst_vma,
>> struct vm_area_struct *src_vma)
>> static inline bool should_zap_cows(struct zap_details *details)
>
> Not sure if you fix up later, but we should probably change this function to
> should_skip_cows() to keep everything consistent, otherwise this is a bit
> weird
> and confusing.
should_skip_cows() is a bit misleading on its own IMHO, as we skip the
"zap" context.
Would have to be something like "should_skip_cows_when_zapping()", and I
am not sure if that's really worth it.
So I think we can leave it as is.
Thanks!
>
>> {
>> /* By default, zap all pages */
>> - if (!details || details->reclaim_pt)
>> + if (!details)
>> return true;
>>
>> + VM_WARN_ON_ONCE(details->skip_cows && details->reclaim_pt);
>> +
>> /* Or, we zap COWed pages only if the caller wants to */
>> - return details->even_cows;
>> + return !details->skip_cows;
>> }
>>
>> /* Decides whether we should zap this folio with the folio pointer
>> specified */
>> @@ -2149,8 +2151,6 @@ void unmap_vmas(struct mmu_gather *tlb, struct
>> unmap_desc *unmap)
>> struct mmu_notifier_range range;
>> struct zap_details details = {
>> .zap_flags = ZAP_FLAG_DROP_MARKER | ZAP_FLAG_UNMAP,
>> - /* Careful - we need to zap private pages too! */
>> - .even_cows = true,
>> };
>>
>> vma = unmap->first;
>> @@ -4282,7 +4282,7 @@ void unmap_mapping_folio(struct folio *folio)
>> first_index = folio->index;
>> last_index = folio_next_index(folio) - 1;
>>
>> - details.even_cows = false;
>> + details.skip_cows = true;
>> details.single_folio = folio;
>> details.zap_flags = ZAP_FLAG_DROP_MARKER;
>>
>> @@ -4312,7 +4312,7 @@ void unmap_mapping_pages(struct address_space
>> *mapping, pgoff_t start,
>> pgoff_t first_index = start;
>> pgoff_t last_index = start + nr - 1;
>>
>> - details.even_cows = even_cows;
>> + details.skip_cows = !even_cows;
>
> Not sure if you clean up later, but seems sensible to cascade the change into
> the local boolean here.
There are too many unmap_mapping_range() users for me to want to change
the external interface :)
What I was thinking is that probably there should be two independent
user-facing functions, instead of having this magical bool passed around
Because I am sure, most users don't really know whether to set it to
true or false ...
But that's something for another cleanup. (I left the whole
unmap_mapping_pages() interface etc alone in this series)
--
Cheers,
David
