2022-03-31 03:08:56

by Fabio M. De Francesco

Subject: [PATCH] staging: vc04_services: Convert kmap() to kmap_local_page()

The use of kmap() is being deprecated in favor of kmap_local_page()
where it is feasible. In file interface/vchiq_arm/vchiq_arm.c,
function free_pagelist() calls kmap() / kunmap() from two places.

With kmap_local_page(), the mapping is per thread, CPU local and not
globally visible. Therefore, free_pagelist() is a function where the
use of kmap_local_page() in place of kmap() is correctly suited.

Convert to kmap_local_page() but, instead of open coding it, use the
memcpy_to_page() helper.

Signed-off-by: Fabio M. De Francesco <[email protected]>
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index f0bfacfdea80..efb1383b5218 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -431,21 +431,18 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
if (head_bytes > actual)
head_bytes = actual;

- memcpy((char *)kmap(pages[0]) +
+ memcpy_to_page(pages[0],
pagelist->offset,
fragments,
head_bytes);
- kunmap(pages[0]);
}
if ((actual >= 0) && (head_bytes < actual) &&
- (tail_bytes != 0)) {
- memcpy((char *)kmap(pages[num_pages - 1]) +
- ((pagelist->offset + actual) &
- (PAGE_SIZE - 1) & ~(g_cache_line_size - 1)),
+ (tail_bytes != 0))
+ memcpy_to_page(pages[num_pages - 1],
+ (pagelist->offset + actual) &
+ (PAGE_SIZE - 1) & ~(g_cache_line_size - 1),
fragments + g_cache_line_size,
tail_bytes);
- kunmap(pages[num_pages - 1]);
- }

down(&g_free_fragments_mutex);
*(char **)fragments = g_free_fragments;
--
2.34.1

2022-04-01 12:36:40

by Greg Kroah-Hartman

Subject: Re: [PATCH] staging: vc04_services: Convert kmap() to kmap_local_page()

On Fri, Apr 01, 2022 at 10:07:36AM +0200, Fabio M. De Francesco wrote:
> On mercoled? 30 marzo 2022 21:14:14 CEST Fabio M. De Francesco wrote:
> > The use of kmap() is being deprecated in favor of kmap_local_page()
> > where it is feasible. In file interface/vchiq_arm/vchiq_arm.c,
> > function free_pagelist() calls kmap() / kunmap() from two places.
> >
> > With kmap_local_page(), the mapping is per thread, CPU local and not
> > globally visible. Therefore, free_pagelist() is a function where the
> > use of kmap_local_page() in place of kmap() is correctly suited.
> >
> > Convert to kmap_local_page() but, instead of open coding it, use the
> > memcpy_to_page() helper.
> >
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
> > ---
> > .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 13 +++++--------
> > 1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index f0bfacfdea80..efb1383b5218 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -431,21 +431,18 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> > if (head_bytes > actual)
> > head_bytes = actual;
> >
> > - memcpy((char *)kmap(pages[0]) +
> > + memcpy_to_page(pages[0],
> > pagelist->offset,
> > fragments,
> > head_bytes);
> > - kunmap(pages[0]);
> > }
> > if ((actual >= 0) && (head_bytes < actual) &&
> > - (tail_bytes != 0)) {
> > - memcpy((char *)kmap(pages[num_pages - 1]) +
> > - ((pagelist->offset + actual) &
> > - (PAGE_SIZE - 1) & ~(g_cache_line_size - 1)),
> > + (tail_bytes != 0))
> > + memcpy_to_page(pages[num_pages - 1],
> > + (pagelist->offset + actual) &
> > + (PAGE_SIZE - 1) & ~(g_cache_line_size - 1),
> > fragments + g_cache_line_size,
> > tail_bytes);
> > - kunmap(pages[num_pages - 1]);
> > - }
> >
> > down(&g_free_fragments_mutex);
> > *(char **)fragments = g_free_fragments;
> > --
> > 2.34.1
> >
> Hi Greg,
>
> I've just received a message from you that says that a patch that I sent
> on March 31 has been applied to staging testing. I know that you usually
> apply patches in first come first served fashion (FIFO), therefore I wonder
> why this patch has not yet been applied.
>
> Please don't misunderstand me: I have no hurry. I'm asking only because
> I suspect that this patch, sent on March 30th) could have been overlooked
> since it has the very identical subject of another patch that I sent on
> the same day (or the day before, I'm not sure about it now) and which has
> already been applied. Therefore, they may appear to be the same patch,
> because the only difference is that the drivers are different.

I wanted to give others the chance to review this before applying it :)

2022-04-02 13:24:20

by Fabio M. De Francesco

Subject: Re: [PATCH] staging: vc04_services: Convert kmap() to kmap_local_page()

On mercoled? 30 marzo 2022 21:14:14 CEST Fabio M. De Francesco wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page()
> where it is feasible. In file interface/vchiq_arm/vchiq_arm.c,
> function free_pagelist() calls kmap() / kunmap() from two places.
>
> With kmap_local_page(), the mapping is per thread, CPU local and not
> globally visible. Therefore, free_pagelist() is a function where the
> use of kmap_local_page() in place of kmap() is correctly suited.
>
> Convert to kmap_local_page() but, instead of open coding it, use the
> memcpy_to_page() helper.
>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index f0bfacfdea80..efb1383b5218 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -431,21 +431,18 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> if (head_bytes > actual)
> head_bytes = actual;
>
> - memcpy((char *)kmap(pages[0]) +
> + memcpy_to_page(pages[0],
> pagelist->offset,
> fragments,
> head_bytes);
> - kunmap(pages[0]);
> }
> if ((actual >= 0) && (head_bytes < actual) &&
> - (tail_bytes != 0)) {
> - memcpy((char *)kmap(pages[num_pages - 1]) +
> - ((pagelist->offset + actual) &
> - (PAGE_SIZE - 1) & ~(g_cache_line_size - 1)),
> + (tail_bytes != 0))
> + memcpy_to_page(pages[num_pages - 1],
> + (pagelist->offset + actual) &
> + (PAGE_SIZE - 1) & ~(g_cache_line_size - 1),
> fragments + g_cache_line_size,
> tail_bytes);
> - kunmap(pages[num_pages - 1]);
> - }
>
> down(&g_free_fragments_mutex);
> *(char **)fragments = g_free_fragments;
> --
> 2.34.1
>
Hi Greg,

I've just received a message from you that says that a patch that I sent
on March 31 has been applied to staging testing. I know that you usually
apply patches in first come first served fashion (FIFO), therefore I wonder
why this patch has not yet been applied.

Please don't misunderstand me: I have no hurry. I'm asking only because
I suspect that this patch, sent on March 30th) could have been overlooked
since it has the very identical subject of another patch that I sent on
the same day (or the day before, I'm not sure about it now) and which has
already been applied. Therefore, they may appear to be the same patch,
because the only difference is that the drivers are different.

Thanks,

Fabio

2022-04-04 23:46:51

by Ira Weiny

Subject: Re: [PATCH] staging: vc04_services: Convert kmap() to kmap_local_page()

On Wed, Mar 30, 2022 at 09:14:14PM +0200, Fabio M. De Francesco wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page()
> where it is feasible. In file interface/vchiq_arm/vchiq_arm.c,
> function free_pagelist() calls kmap() / kunmap() from two places.
>
> With kmap_local_page(), the mapping is per thread, CPU local and not
> globally visible. Therefore, free_pagelist() is a function where the
> use of kmap_local_page() in place of kmap() is correctly suited.
>
> Convert to kmap_local_page() but, instead of open coding it, use the
> memcpy_to_page() helper.
>
> Signed-off-by: Fabio M. De Francesco <[email protected]>

LGTM:

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

> ---
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index f0bfacfdea80..efb1383b5218 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -431,21 +431,18 @@ free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
> if (head_bytes > actual)
> head_bytes = actual;
>
> - memcpy((char *)kmap(pages[0]) +
> + memcpy_to_page(pages[0],
> pagelist->offset,
> fragments,
> head_bytes);
> - kunmap(pages[0]);
> }
> if ((actual >= 0) && (head_bytes < actual) &&
> - (tail_bytes != 0)) {
> - memcpy((char *)kmap(pages[num_pages - 1]) +
> - ((pagelist->offset + actual) &
> - (PAGE_SIZE - 1) & ~(g_cache_line_size - 1)),
> + (tail_bytes != 0))
> + memcpy_to_page(pages[num_pages - 1],
> + (pagelist->offset + actual) &
> + (PAGE_SIZE - 1) & ~(g_cache_line_size - 1),
> fragments + g_cache_line_size,
> tail_bytes);
> - kunmap(pages[num_pages - 1]);
> - }
>
> down(&g_free_fragments_mutex);
> *(char **)fragments = g_free_fragments;
> --
> 2.34.1
>