[PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page

Deepak R Varma posted 6 patches 2 years, 7 months ago
[PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page
Posted by Deepak R Varma 2 years, 7 months ago
kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().

Therefore, replace kmap_atomic() with kmap_local_page() in
gfs2_write_buf_to_page().

kmap_atomic() disables page-faults and preemption (the latter only for
!PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
gfs2_write_buf_to_page() does not depend on the above-mentioned side
effects.

Therefore, a mere replacement of the old API with the new one is all that
is required (i.e., there is no need to explicitly add any calls to
pagefault_disable() and/or preempt_disable()).

Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Deepak R Varma <drv@mailo.com>
---
Changes in v3:
   - Patch included in patch set

Changes in v2:
   - None


 fs/gfs2/quota.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 386ca770ce2e..e5767133aeea 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -764,10 +764,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode *ip, unsigned long index,
 	}
 
 	/* Write to the page, now that we have setup the buffer(s) */
-	kaddr = kmap_atomic(page);
+	kaddr = kmap_local_page(page);
 	memcpy(kaddr + off, buf, bytes);
 	flush_dcache_page(page);
-	kunmap_atomic(kaddr);
+	kunmap_local(kaddr);
 	unlock_page(page);
 	put_page(page);
 
-- 
2.34.1
Re: [PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page
Posted by Fabio M. De Francesco 2 years, 7 months ago
On giovedì 29 giugno 2023 23:52:27 CEST Deepak R Varma wrote:
> kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().

Deepak,

Again please refer to documentation and/or Ira's deprecation patch. The 
reasons why are in one of my previous messages.

> Therefore, replace kmap_atomic() with kmap_local_page() in
> gfs2_write_buf_to_page().
> 
> kmap_atomic() disables page-faults and preemption (the latter only for
> !PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
> gfs2_write_buf_to_page() does not depend on the above-mentioned side
> effects.
> 
> Therefore, a mere replacement of the old API with the new one is all that
> is required (i.e., there is no need to explicitly add any calls to
> pagefault_disable() and/or preempt_disable()).
>
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> Changes in v3:
>    - Patch included in patch set
> 
> Changes in v2:
>    - None
> 
> 
>  fs/gfs2/quota.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index 386ca770ce2e..e5767133aeea 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -764,10 +764,10 @@ static int gfs2_write_buf_to_page(struct gfs2_inode 
*ip,
> unsigned long index, }
> 
>  	/* Write to the page, now that we have setup the buffer(s) */
> -	kaddr = kmap_atomic(page);
> +	kaddr = kmap_local_page(page);
>
Well, if this page could come from HIGHMEM, how about memcpy_to_page()? 
Otherwise, (if it cannot come from HIGHMEM) we don't need to kmap*() it. 

Can you please take a look at the allocation's flags?

Thanks,

Fabio
>
>  	memcpy(kaddr + off, buf, bytes);
>  	flush_dcache_page(page);
> -	kunmap_atomic(kaddr);
> +	kunmap_local(kaddr);
>  	unlock_page(page);
>  	put_page(page);
> 
> --
> 2.34.1
Re: [PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page
Posted by Deepak R Varma 2 years, 6 months ago
On Sat, Jul 01, 2023 at 03:54:06PM +0200, Fabio M. De Francesco wrote:
> On giovedì 29 giugno 2023 23:52:27 CEST Deepak R Varma wrote:
> > kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
>
> Deepak,
>
> Again please refer to documentation and/or Ira's deprecation patch. The
> reasons why are in one of my previous messages.

Hi Fabio,
This change was already added by Andreas. So my patchset can be dropped.
However, your feedback on the individual patches is agreed to and accepted. I
will keep your suggestions in mind when I submit next patches.

Thank you :)

Deepak.

>
> > Therefore, replace kmap_atomic() with kmap_local_page() in
> > --
> > 2.34.1
>
>
>
>