Hi Ira,
On 2/27/2023 1:31 PM, Ira Weiny wrote:
> 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@xxxxxxxxx>
>> ---
>> 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/
Thanks for pointing this out. I got the build error report from LKP also.
>
> 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.
Yes. Totally agree.
Regards
Yin, Fengwei
>
> 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
>>
>>
>
>
