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

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

[Fabio M. De Francesco]

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 <fmdefrancesco@gmail.com>
---
 .../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

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

[Fabio M. De Francesco]

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 <fmdefrancesco@gmail.com>
> ---
>  .../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

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

[Greg Kroah-Hartman]

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 <fmdefrancesco@gmail.com>
> > ---
> >  .../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 :)

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

[Ira Weiny]

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 <fmdefrancesco@gmail.com>

LGTM:

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

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