diff mbox series

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

Message ID 4bed561513ba76486ea3fc87f97e6c646f98cbe7.1688073459.git.drv@mailo.com
State New
Headers
Series gfs2: kmap{_atomic} conversion to kmap_local_{page/folio} |

Commit Message

Deepak R Varma June 29, 2023, 9:52 p.m. UTC 
  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(-)

Comments

Fabio M. De Francesco July 1, 2023, 1:54 p.m. UTC | #1 
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
Deepak R Varma Aug. 10, 2023, 3:28 p.m. UTC | #2 
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
>
>
>
>
diff mbox series

Patch

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