[PATCH v1 09/16] mm/memory: convert details->even_cows into details->skip_cows

[David Hildenbrand (Arm)]

>>
>>  	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