2021-02-04 07:36:07

by Prathu Baronia

[permalink] [raw]
Subject: [PATCH v4 0/1] mm/highmem: Remove deprecated kmap_atomic

Updated the commit text.

Changelog:
Added my test data to Ira's v3 patch.

Ira Weiny (1):
mm/highmem: Remove deprecated kmap_atomic

include/linux/highmem.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

--
2.17.1


2021-02-04 23:30:17

by Prathu Baronia

[permalink] [raw]
Subject: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

From: Ira Weiny <[email protected]>

kmap_atomic() is being deprecated in favor of kmap_local_page().

Replace the uses of kmap_atomic() within the highmem code.

On profiling clear_huge_page() using ftrace an improvement
of 62% was observed on the below setup.

Setup:-
Below data has been collected on Qualcomm's SM7250 SoC THP enabled
(kernel v4.19.113) with only CPU-0(Cortex-A55) and CPU-7(Cortex-A76)
switched on and set to max frequency, also DDR set to perf governor.

FTRACE Data:-

Base data:-
Number of iterations: 48
Mean of allocation time: 349.5 us
std deviation: 74.5 us

v4 data:-
Number of iterations: 48
Mean of allocation time: 131 us
std deviation: 32.7 us

The following simple userspace experiment to allocate
100MB(BUF_SZ) of pages and writing to it gave us a good insight,
we observed an improvement of 42% in allocation and writing timings.
-------------------------------------------------------------
Test code snippet
-------------------------------------------------------------
clock_start();
buf = malloc(BUF_SZ); /* Allocate 100 MB of memory */

for(i=0; i < BUF_SZ_PAGES; i++)
{
*((int *)(buf + (i*PAGE_SIZE))) = 1;
}
clock_end();
-------------------------------------------------------------

Malloc test timings for 100MB anon allocation:-

Base data:-
Number of iterations: 100
Mean of allocation time: 31831 us
std deviation: 4286 us

v4 data:-
Number of iterations: 100
Mean of allocation time: 18193 us
std deviation: 4915 us

Signed-off-by: Ira Weiny <[email protected]>
Signed-off-by: Prathu Baronia <[email protected]>
[Updated commit text with test data]
---
include/linux/highmem.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index d2c70d3772a3..9a202c7e4e26 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -146,9 +146,9 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
#ifndef clear_user_highpage
static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
{
- void *addr = kmap_atomic(page);
+ void *addr = kmap_local_page(page);
clear_user_page(addr, vaddr, page);
- kunmap_atomic(addr);
+ kunmap_local(addr);
}
#endif

@@ -199,9 +199,9 @@ alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,

static inline void clear_highpage(struct page *page)
{
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
clear_page(kaddr);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}

/*
@@ -216,7 +216,7 @@ static inline void zero_user_segments(struct page *page,
unsigned start1, unsigned end1,
unsigned start2, unsigned end2)
{
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
unsigned int i;

BUG_ON(end1 > page_size(page) || end2 > page_size(page));
@@ -227,7 +227,7 @@ static inline void zero_user_segments(struct page *page,
if (end2 > start2)
memset(kaddr + start2, 0, end2 - start2);

- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
for (i = 0; i < compound_nr(page); i++)
flush_dcache_page(page + i);
}
@@ -252,11 +252,11 @@ static inline void copy_user_highpage(struct page *to, struct page *from,
{
char *vfrom, *vto;

- vfrom = kmap_atomic(from);
- vto = kmap_atomic(to);
+ vfrom = kmap_local_page(from);
+ vto = kmap_local_page(to);
copy_user_page(vto, vfrom, vaddr, to);
- kunmap_atomic(vto);
- kunmap_atomic(vfrom);
+ kunmap_local(vto);
+ kunmap_local(vfrom);
}

#endif
@@ -267,11 +267,11 @@ static inline void copy_highpage(struct page *to, struct page *from)
{
char *vfrom, *vto;

- vfrom = kmap_atomic(from);
- vto = kmap_atomic(to);
+ vfrom = kmap_local_page(from);
+ vto = kmap_local_page(to);
copy_page(vto, vfrom);
- kunmap_atomic(vto);
- kunmap_atomic(vfrom);
+ kunmap_local(vto);
+ kunmap_local(vfrom);
}

#endif
--
2.17.1

2021-02-11 00:35:46

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Thu, Feb 04, 2021 at 01:02:53PM +0530, Prathu Baronia wrote:
> From: Ira Weiny <[email protected]>
>
> kmap_atomic() is being deprecated in favor of kmap_local_page().
>
> Replace the uses of kmap_atomic() within the highmem code.
>
> On profiling clear_huge_page() using ftrace an improvement
> of 62% was observed on the below setup.
>
> Setup:-
> Below data has been collected on Qualcomm's SM7250 SoC THP enabled
> (kernel v4.19.113) with only CPU-0(Cortex-A55) and CPU-7(Cortex-A76)
> switched on and set to max frequency, also DDR set to perf governor.
>
> FTRACE Data:-
>
> Base data:-
> Number of iterations: 48
> Mean of allocation time: 349.5 us
> std deviation: 74.5 us
>
> v4 data:-
> Number of iterations: 48
> Mean of allocation time: 131 us
> std deviation: 32.7 us
>
> The following simple userspace experiment to allocate
> 100MB(BUF_SZ) of pages and writing to it gave us a good insight,
> we observed an improvement of 42% in allocation and writing timings.
> -------------------------------------------------------------
> Test code snippet
> -------------------------------------------------------------
> clock_start();
> buf = malloc(BUF_SZ); /* Allocate 100 MB of memory */
>
> for(i=0; i < BUF_SZ_PAGES; i++)
> {
> *((int *)(buf + (i*PAGE_SIZE))) = 1;
> }
> clock_end();
> -------------------------------------------------------------
>
> Malloc test timings for 100MB anon allocation:-
>
> Base data:-
> Number of iterations: 100
> Mean of allocation time: 31831 us
> std deviation: 4286 us
>
> v4 data:-
> Number of iterations: 100
> Mean of allocation time: 18193 us
> std deviation: 4915 us
>
> Signed-off-by: Ira Weiny <[email protected]>

This already has my signed off by so I'm not going to 'review'. With Prathu's
testing information I hope this can land.

Andrew did you see this patch?

Thanks,
Ira

> Signed-off-by: Prathu Baronia <[email protected]>
> [Updated commit text with test data]
> ---
> include/linux/highmem.h | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index d2c70d3772a3..9a202c7e4e26 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -146,9 +146,9 @@ static inline void invalidate_kernel_vmap_range(void *vaddr, int size)
> #ifndef clear_user_highpage
> static inline void clear_user_highpage(struct page *page, unsigned long vaddr)
> {
> - void *addr = kmap_atomic(page);
> + void *addr = kmap_local_page(page);
> clear_user_page(addr, vaddr, page);
> - kunmap_atomic(addr);
> + kunmap_local(addr);
> }
> #endif
>
> @@ -199,9 +199,9 @@ alloc_zeroed_user_highpage_movable(struct vm_area_struct *vma,
>
> static inline void clear_highpage(struct page *page)
> {
> - void *kaddr = kmap_atomic(page);
> + void *kaddr = kmap_local_page(page);
> clear_page(kaddr);
> - kunmap_atomic(kaddr);
> + kunmap_local(kaddr);
> }
>
> /*
> @@ -216,7 +216,7 @@ static inline void zero_user_segments(struct page *page,
> unsigned start1, unsigned end1,
> unsigned start2, unsigned end2)
> {
> - void *kaddr = kmap_atomic(page);
> + void *kaddr = kmap_local_page(page);
> unsigned int i;
>
> BUG_ON(end1 > page_size(page) || end2 > page_size(page));
> @@ -227,7 +227,7 @@ static inline void zero_user_segments(struct page *page,
> if (end2 > start2)
> memset(kaddr + start2, 0, end2 - start2);
>
> - kunmap_atomic(kaddr);
> + kunmap_local(kaddr);
> for (i = 0; i < compound_nr(page); i++)
> flush_dcache_page(page + i);
> }
> @@ -252,11 +252,11 @@ static inline void copy_user_highpage(struct page *to, struct page *from,
> {
> char *vfrom, *vto;
>
> - vfrom = kmap_atomic(from);
> - vto = kmap_atomic(to);
> + vfrom = kmap_local_page(from);
> + vto = kmap_local_page(to);
> copy_user_page(vto, vfrom, vaddr, to);
> - kunmap_atomic(vto);
> - kunmap_atomic(vfrom);
> + kunmap_local(vto);
> + kunmap_local(vfrom);
> }
>
> #endif
> @@ -267,11 +267,11 @@ static inline void copy_highpage(struct page *to, struct page *from)
> {
> char *vfrom, *vto;
>
> - vfrom = kmap_atomic(from);
> - vto = kmap_atomic(to);
> + vfrom = kmap_local_page(from);
> + vto = kmap_local_page(to);
> copy_page(vto, vfrom);
> - kunmap_atomic(vto);
> - kunmap_atomic(vfrom);
> + kunmap_local(vto);
> + kunmap_local(vfrom);
> }
>
> #endif
> --
> 2.17.1
>

2021-02-11 23:58:03

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:

> > Signed-off-by: Ira Weiny <[email protected]>
>
> This already has my signed off by so I'm not going to 'review'. With Prathu's
> testing information I hope this can land.
>
> Andrew did you see this patch?

I did now ;)

Tossed onto the post-rc1 pile, thanks,

2021-11-05 16:10:31

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
>
> > > Signed-off-by: Ira Weiny <[email protected]>
> >
> > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > testing information I hope this can land.
> >
> > Andrew did you see this patch?
>
> I did now ;)
>
> Tossed onto the post-rc1 pile, thanks,

This patch seems to have slipped through the gaps for a couple of cycles
now? I found a missed spot in it for CONFIG_HIGHMEM:

diff --git a/mm/highmem.c b/mm/highmem.c
index 471d9779a7f4..82d8c5ab6e8d 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -382,7 +382,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
unsigned this_end = min_t(unsigned, end1, PAGE_SIZE);

if (end1 > start1) {
- kaddr = kmap_atomic(page + i);
+ kaddr = kmap_local(page + i);
memset(kaddr + start1, 0, this_end - start1);
}
end1 -= this_end;
@@ -397,7 +397,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,

if (end2 > start2) {
if (!kaddr)
- kaddr = kmap_atomic(page + i);
+ kaddr = kmap_local(page + i);
memset(kaddr + start2, 0, this_end - start2);
}
end2 -= this_end;
@@ -405,7 +405,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
}

if (kaddr) {
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
flush_dcache_page(page + i);
}

2021-11-05 17:37:30

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 09:58:59AM -0700, Ira Weiny wrote:
> On Fri, Nov 05, 2021 at 04:51:40PM +0000, Matthew Wilcox wrote:
> > On Fri, Nov 05, 2021 at 01:50:37PM +0000, Matthew Wilcox wrote:
> > > On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > > > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> > > >
> > > > > > Signed-off-by: Ira Weiny <[email protected]>
> > > > >
> > > > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > > > testing information I hope this can land.
> > > > >
> > > > > Andrew did you see this patch?
> > > >
> > > > I did now ;)
> > > >
> > > > Tossed onto the post-rc1 pile, thanks,
> > >
> > > This patch seems to have slipped through the gaps for a couple of cycles
> > > now? I found a missed spot in it for CONFIG_HIGHMEM:
> >
> > Ugh, sorry, wrong version of the patch.
>
> Check! Yea this works for me...
>
> I think this should to through as a separate patch because Prathu's has been
> soaking for some time. No need to complicate it with this.

This isn't "complicating Prathu's patch". This is "fixing up the bit
that Prathu missed with his patch". zero_user_segments() should not
have different rules on HIGHMEM and non-HIGHMEM kernels.

2021-11-05 18:37:22

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 01:49:21PM +0000, Matthew Wilcox wrote:
> On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> >
> > > > Signed-off-by: Ira Weiny <[email protected]>
> > >
> > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > testing information I hope this can land.
> > >
> > > Andrew did you see this patch?
> >
> > I did now ;)
> >
> > Tossed onto the post-rc1 pile, thanks,
>
> This patch seems to have slipped through the gaps for a couple of cycles
> now? I found a missed spot in it for CONFIG_HIGHMEM:
>
> diff --git a/mm/highmem.c b/mm/highmem.c
> index 471d9779a7f4..82d8c5ab6e8d 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -382,7 +382,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
> unsigned this_end = min_t(unsigned, end1, PAGE_SIZE);
>
> if (end1 > start1) {
> - kaddr = kmap_atomic(page + i);
> + kaddr = kmap_local(page + i);

kmap_local_page()

> memset(kaddr + start1, 0, this_end - start1);
> }
> end1 -= this_end;
> @@ -397,7 +397,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
>
> if (end2 > start2) {
> if (!kaddr)
> - kaddr = kmap_atomic(page + i);
> + kaddr = kmap_local(page + i);

kmap_local_page()

I'm ok with this. I'm not sure if Prathu needed this or not.

Also I wonder if memset_page could be used? It would end up mapping the page
2x sometimes.

As an aside I think flush_dcache_page() needs to be in memset_page() for
completeness but I'm a bit afraid of adding it with the current controversy...
:-/

Anyway, why hasn't Brathu's patch[1] landed yet?

I just checked 5.15 and it is not there? I don't even see it in the 5.16
merge? Perhaps it is coming in this merge window? Andrew?

Ira

[1] https://lore.kernel.org/lkml/[email protected]/

> memset(kaddr + start2, 0, this_end - start2);
> }
> end2 -= this_end;
> @@ -405,7 +405,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
> }
>
> if (kaddr) {
> - kunmap_atomic(kaddr);
> + kunmap_local(kaddr);
> flush_dcache_page(page + i);
> }


2021-11-05 18:53:43

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 01:50:37PM +0000, Matthew Wilcox wrote:
> On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> >
> > > > Signed-off-by: Ira Weiny <[email protected]>
> > >
> > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > testing information I hope this can land.
> > >
> > > Andrew did you see this patch?
> >
> > I did now ;)
> >
> > Tossed onto the post-rc1 pile, thanks,
>
> This patch seems to have slipped through the gaps for a couple of cycles
> now? I found a missed spot in it for CONFIG_HIGHMEM:

Ugh, sorry, wrong version of the patch.

---
mm/highmem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/highmem.c b/mm/highmem.c
index 471d9779a7f4..88f65f155845 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -382,7 +382,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
unsigned this_end = min_t(unsigned, end1, PAGE_SIZE);

if (end1 > start1) {
- kaddr = kmap_atomic(page + i);
+ kaddr = kmap_local_page(page + i);
memset(kaddr + start1, 0, this_end - start1);
}
end1 -= this_end;
@@ -397,7 +397,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,

if (end2 > start2) {
if (!kaddr)
- kaddr = kmap_atomic(page + i);
+ kaddr = kmap_local_page(page + i);
memset(kaddr + start2, 0, this_end - start2);
}
end2 -= this_end;
@@ -405,7 +405,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
}

if (kaddr) {
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
flush_dcache_page(page + i);
}

--
2.33.0

2021-11-05 18:59:21

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 04:51:40PM +0000, Matthew Wilcox wrote:
> On Fri, Nov 05, 2021 at 01:50:37PM +0000, Matthew Wilcox wrote:
> > On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> > >
> > > > > Signed-off-by: Ira Weiny <[email protected]>
> > > >
> > > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > > testing information I hope this can land.
> > > >
> > > > Andrew did you see this patch?
> > >
> > > I did now ;)
> > >
> > > Tossed onto the post-rc1 pile, thanks,
> >
> > This patch seems to have slipped through the gaps for a couple of cycles
> > now? I found a missed spot in it for CONFIG_HIGHMEM:
>
> Ugh, sorry, wrong version of the patch.

Check! Yea this works for me...

I think this should to through as a separate patch because Prathu's has been
soaking for some time. No need to complicate it with this.

FWIW you can add:

Reviewed-by: Ira Weiny <[email protected]>

When you submit this.

Ira

>
> ---
> mm/highmem.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/highmem.c b/mm/highmem.c
> index 471d9779a7f4..88f65f155845 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -382,7 +382,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
> unsigned this_end = min_t(unsigned, end1, PAGE_SIZE);
>
> if (end1 > start1) {
> - kaddr = kmap_atomic(page + i);
> + kaddr = kmap_local_page(page + i);
> memset(kaddr + start1, 0, this_end - start1);
> }
> end1 -= this_end;
> @@ -397,7 +397,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
>
> if (end2 > start2) {
> if (!kaddr)
> - kaddr = kmap_atomic(page + i);
> + kaddr = kmap_local_page(page + i);
> memset(kaddr + start2, 0, this_end - start2);
> }
> end2 -= this_end;
> @@ -405,7 +405,7 @@ void zero_user_segments(struct page *page, unsigned start1, unsigned end1,
> }
>
> if (kaddr) {
> - kunmap_atomic(kaddr);
> + kunmap_local(kaddr);
> flush_dcache_page(page + i);
> }
>
> --
> 2.33.0
>

2021-11-05 19:28:43

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 09:56:16AM -0700, Ira Weiny wrote:
> Also I wonder if memset_page could be used? It would end up mapping the page
> 2x sometimes.

That was the point of this function existing, to avoid the
double-mapping.

> As an aside I think flush_dcache_page() needs to be in memset_page() for
> completeness but I'm a bit afraid of adding it with the current controversy...
> :-/

Looks like we now have agreement that it does need to be added, and I
agree with you; send a patch.

At some point, I'm probably going to need to foliate the mem*_page
helpers. I've just added:

static inline void folio_zero_segments(struct folio *folio,
size_t start1, size_t end1, size_t start2, size_t end2)
{
zero_user_segments(&folio->page, start1, end1, start2, end2);
}

static inline void folio_zero_segment(struct folio *folio,
size_t start, size_t end)
{
zero_user_segments(&folio->page, start, end, 0, 0);
}

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);
}

but I imagine when we foliate btrfs, we'll need to expand the helpers.
I'll probably do something similar to zero_user_segments; have
out-of-line versions for HIGHMEM and inline versions for !HIGHMEM
(we can do the entire folio with HIGHMEM, but need to go page-by-page
on !HIGHMEM).

2021-11-05 19:46:52

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

On Fri, Nov 05, 2021 at 05:23:11PM +0000, Matthew Wilcox wrote:
> On Fri, Nov 05, 2021 at 09:58:59AM -0700, Ira Weiny wrote:
> > On Fri, Nov 05, 2021 at 04:51:40PM +0000, Matthew Wilcox wrote:
> > > On Fri, Nov 05, 2021 at 01:50:37PM +0000, Matthew Wilcox wrote:
> > > > On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > > > > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> > > > >
> > > > > > > Signed-off-by: Ira Weiny <[email protected]>
> > > > > >
> > > > > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > > > > testing information I hope this can land.
> > > > > >
> > > > > > Andrew did you see this patch?
> > > > >
> > > > > I did now ;)
> > > > >
> > > > > Tossed onto the post-rc1 pile, thanks,
> > > >
> > > > This patch seems to have slipped through the gaps for a couple of cycles
> > > > now? I found a missed spot in it for CONFIG_HIGHMEM:
> > >
> > > Ugh, sorry, wrong version of the patch.
> >
> > Check! Yea this works for me...
> >
> > I think this should to through as a separate patch because Prathu's has been
> > soaking for some time. No need to complicate it with this.
>
> This isn't "complicating Prathu's patch". This is "fixing up the bit
> that Prathu missed with his patch". zero_user_segments() should not
> have different rules on HIGHMEM and non-HIGHMEM kernels.

What do you mean by 'different rules'?

Oh I see... Ok yea.

Well this should not be a big deal...

Ira

2021-11-08 13:49:18

by Prathu Baronia

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] mm/highmem: Remove deprecated kmap_atomic

Hi Mathew,
Sorry for the late reply, I was on vacation. I can see that you
already fixed up the patch(added the zero_user_segments part) and the
patch[1] is now merged in the mainline. Thanks for fixing it up. Also
for the memset_page() thing, IIUC would the below diff suffice?

diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 25aff0f2ed0b..dfe21e6a696b 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -332,6 +332,7 @@ static inline void memset_page(struct page *page,
size_t offset, int val,

VM_BUG_ON(offset + len > PAGE_SIZE);
memset(addr + offset, val, len);
+ flush_dcache_page(page);
kunmap_local(addr);
}


- Prathu

[1]: https://github.com/torvalds/linux/commit/d2c20e51e3966bc668ef1ef21fbe90704286c8d0#diff-a5b54499adfa477c1341f1d74cb5331481f00ff9afb4c8d600be19e2d8fa7e30

On Sat, Nov 6, 2021 at 12:08 AM Ira Weiny <[email protected]> wrote:
>
> On Fri, Nov 05, 2021 at 05:23:11PM +0000, Matthew Wilcox wrote:
> > On Fri, Nov 05, 2021 at 09:58:59AM -0700, Ira Weiny wrote:
> > > On Fri, Nov 05, 2021 at 04:51:40PM +0000, Matthew Wilcox wrote:
> > > > On Fri, Nov 05, 2021 at 01:50:37PM +0000, Matthew Wilcox wrote:
> > > > > On Thu, Feb 11, 2021 at 03:56:25PM -0800, Andrew Morton wrote:
> > > > > > On Wed, 10 Feb 2021 16:33:07 -0800 Ira Weiny <[email protected]> wrote:
> > > > > >
> > > > > > > > Signed-off-by: Ira Weiny <[email protected]>
> > > > > > >
> > > > > > > This already has my signed off by so I'm not going to 'review'. With Prathu's
> > > > > > > testing information I hope this can land.
> > > > > > >
> > > > > > > Andrew did you see this patch?
> > > > > >
> > > > > > I did now ;)
> > > > > >
> > > > > > Tossed onto the post-rc1 pile, thanks,
> > > > >
> > > > > This patch seems to have slipped through the gaps for a couple of cycles
> > > > > now? I found a missed spot in it for CONFIG_HIGHMEM:
> > > >
> > > > Ugh, sorry, wrong version of the patch.
> > >
> > > Check! Yea this works for me...
> > >
> > > I think this should to through as a separate patch because Prathu's has been
> > > soaking for some time. No need to complicate it with this.
> >
> > This isn't "complicating Prathu's patch". This is "fixing up the bit
> > that Prathu missed with his patch". zero_user_segments() should not
> > have different rules on HIGHMEM and non-HIGHMEM kernels.
>
> What do you mean by 'different rules'?
>
> Oh I see... Ok yea.
>
> Well this should not be a big deal...
>
> Ira