Re: [RFC PATCH 2/2] mm: add zero_user_folio_segments()

[Ira Weiny <ira.weiny@intel.com>]

Yin Fengwei wrote:
> zero_user_folio_segments() has same function as zero_user_segments().
> but take folio as parameter. Update folio_zero_segments(),
> folio_zero_segment() and folio_zero_range() to use it.
> 
> Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
> ---
>  include/linux/highmem.h | 26 +++++++++++++++++---
>  mm/highmem.c            | 53 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index b06254e76d99..0039116e416a 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -266,6 +266,8 @@ static inline void tag_clear_highpage(struct page *page)
>  #ifdef CONFIG_HIGHMEM
>  void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
>  		unsigned start2, unsigned end2);
> +void zero_user_folio_segments(struct folio *folio, unsigned start1,
> +		unsigned end1, unsigned start2, unsigned end2);
>  #else
>  static inline void zero_user_segments(struct page *page,
>  		unsigned start1, unsigned end1,
> @@ -286,6 +288,24 @@ static inline void zero_user_segments(struct page *page,
>  	for (i = 0; i < compound_nr(page); i++)
>  		flush_dcache_page(page + i);
>  }
> +
> +static inline void zero_user_folio_segments(struct folio *folio,
> +		unsigned start1, unsigned end1,
> +		unsigned start2, unsigned end2)
> +{
> +	void *kaddr = kmap_local_page(&folio->page);
> +
> +	BUG_ON(end1 > folio_size(folio) || end2 > folio_size(folio));
> +
> +	if (end1 > start1)
> +		memset(kaddr + start1, 0, end1 - start1);
> +
> +	if (end2 > start2)
> +		memset(kaddr + start2, 0, end2 - start2);
> +
> +	kunmap_local(kaddr);
> +	flush_dcache_folio(folio);
> +}
>  #endif
>  
>  static inline void zero_user_segment(struct page *page,
> @@ -454,7 +474,7 @@ static inline size_t memcpy_from_file_folio(char *to, struct folio *folio,
>  static inline void folio_zero_segments(struct folio *folio,
>  		size_t start1, size_t xend1, size_t start2, size_t xend2)
>  {
> -	zero_user_segments(&folio->page, start1, xend1, start2, xend2);
> +	zero_user_folio_segments(folio, start1, xend1, start2, xend2);
>  }
>  
>  /**
> @@ -466,7 +486,7 @@ static inline void folio_zero_segments(struct folio *folio,
>  static inline void folio_zero_segment(struct folio *folio,
>  		size_t start, size_t xend)
>  {
> -	zero_user_segments(&folio->page, start, xend, 0, 0);
> +	zero_user_folio_segments(folio, start, xend, 0, 0);
>  }
>  
>  /**
> @@ -478,7 +498,7 @@ static inline void folio_zero_segment(struct folio *folio,
>  static inline void folio_zero_range(struct folio *folio,
>  		size_t start, size_t length)
>  {
> -	zero_user_segments(&folio->page, start, start + length, 0, 0);
> +	zero_user_folio_segments(folio, start, start + length, 0, 0);
>  }
>  
>  #endif /* _LINUX_HIGHMEM_H */
> diff --git a/mm/highmem.c b/mm/highmem.c
> index db251e77f98f..e234b249208f 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -443,6 +443,59 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
>  	BUG_ON((start1 | start2 | end1 | end2) != 0);
>  }
>  EXPORT_SYMBOL(zero_user_segments);
> +
> +static inline void zero_user_folio_segment(struct folio *folio,

FWIW this does not compile:

s/zero_user_folio_segment/zero_user_folio_segments/

But I agree with Willy here that I don't see the point of this patch.
Seems like a lot of extra code for no benefit.

Ira


> +		unsigned start1, unsigned end1,
> +		unsigned start2, unsigned end2)
> +{
> +	void *kaddr;
> +	unsigned s;
> +
> +	BUG_ON(end1 > folio_size(folio) || end2 > folio_size(folio));
> +
> +	if (start1 > start2) {
> +		swap(start1, start2);
> +		swap(end1, end2);
> +	}
> +
> +	if (start1 >= end1)
> +		start1 = end1 = 0;
> +
> +	if (start2 >= end2)
> +		start2 = end2 = 0;
> +
> +	start2 = max_t(unsigned, end1, start2);
> +	s = start1;
> +	while((start1 < end1) || (start2 < end2)) {
> +		kaddr = kmap_local_folio(folio,
> +				offset_in_folio(folio, PAGE_ALIGN_DOWN(s)));
> +
> +		if ((end2 > start2) && (end1 >= start1)) {
> +			unsigned this_end = min_t(unsigned, end2,
> +					PAGE_ALIGN_DOWN(start2) + PAGE_SIZE);
> +
> +			memset(kaddr + offset_in_page(start2), 0,
> +						this_end - start2);
> +
> +			start2 = this_end;
> +		}
> +
> +		if (end1 > start1) {
> +			unsigned this_end = min_t(unsigned, end1,
> +					PAGE_ALIGN_DOWN(start1) + PAGE_SIZE);
> +
> +			memset(kaddr + offset_in_page(start1), 0,
> +							this_end - start1);
> +			s = start1 = this_end;
> +		} else {
> +			s = start2;
> +		}
> +		kunmap_local(kaddr);
> +	}
> +
> +	flush_dcache_folio(folio);
> +}
> +EXPORT_SYMBOL(zero_user_folio_segments);
>  #endif /* CONFIG_HIGHMEM */
>  
>  #ifdef CONFIG_KMAP_LOCAL
> -- 
> 2.30.2
> 
>