Re: [Cluster-devel] [PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page
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
>
>
>
>
[Cluster-devel] [PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page
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
Signed-off-by: Deepak R Varma
---
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
[Cluster-devel] [PATCH v3 5/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_read_super
The use of kmap() is being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as the mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and still valid. Therefore, replace kmap() with kmap_local_page() in gfs2_read_super(). Suggested-by: Fabio M. De Francesco Signed-off-by: Deepak R Varma --- Changes in v3: - Patch included in patch set Changes in v2: - None fs/gfs2/ops_fstype.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 8a27957dbfee..80fe61662412 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -264,9 +264,9 @@ static int gfs2_read_super(struct gfs2_sbd *sdp, sector_t sector, int silent) __free_page(page); return -EIO; } - p = kmap(page); + p = kmap_local_page(page); gfs2_sb_in(sdp, p); - kunmap(page); + kunmap_local(p); __free_page(page); return gfs2_check_sb(sdp, silent); } -- 2.34.1
[Cluster-devel] [PATCH v3 4/6] gfs2: Replace kmap_atomic() by kmap_local_page() in lops.c
kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
Therefore, replace kmap_atomic() with kmap_local_page() in following
functions of lops.c:
- gfs2_jhead_pg_srch()
- gfs2_check_magic()
- gfs2_before_commit()
kmap_atomic() disables page-faults and preemption (the latter only for
!PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
stuffed_readpage() 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
Signed-off-by: Deepak R Varma
---
Changes in v3:
- Patch included in patch series
Changes in v2:
- None
fs/gfs2/lops.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 1902413d5d12..a7c2296cb3c6 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -427,7 +427,7 @@ static bool gfs2_jhead_pg_srch(struct gfs2_jdesc *jd,
{
struct gfs2_sbd *sdp = GFS2_SB(jd->jd_inode);
struct gfs2_log_header_host lh;
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
unsigned int offset;
bool ret = false;
@@ -441,7 +441,7 @@ static bool gfs2_jhead_pg_srch(struct gfs2_jdesc *jd,
}
}
}
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
return ret;
}
@@ -626,11 +626,11 @@ static void gfs2_check_magic(struct buffer_head *bh)
__be32 *ptr;
clear_buffer_escaped(bh);
- kaddr = kmap_atomic(bh->b_page);
+ kaddr = kmap_local_page(bh->b_page);
ptr = kaddr + bh_offset(bh);
if (*ptr == cpu_to_be32(GFS2_MAGIC))
set_buffer_escaped(bh);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}
static int blocknr_cmp(void *priv, const struct list_head *a,
@@ -699,10 +699,10 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp,
unsigned int limit,
void *kaddr;
page = mempool_alloc(gfs2_page_pool, GFP_NOIO);
ptr = page_address(page);
- kaddr = kmap_atomic(bd2->bd_bh->b_page);
+ kaddr = kmap_local_page(bd2->bd_bh->b_page);
memcpy(ptr, kaddr + bh_offset(bd2->bd_bh),
bd2->bd_bh->b_size);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
*(__be32 *)ptr = 0;
clear_buffer_escaped(bd2->bd_bh);
unlock_buffer(bd2->bd_bh);
--
2.34.1
[Cluster-devel] [PATCH v3 3/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_unstuffer_page
The use of kmap() is being deprecated in favor of kmap_local_page().
There are two main problems with kmap(): (1) It comes with an overhead as
the mapping space is restricted and protected by a global lock for
synchronization and (2) it also requires global TLB invalidation when the
kmap’s pool wraps and it might block when the mapping space is fully
utilized until a slot becomes available.
With kmap_local_page() the mappings are per thread, CPU local, can take
page faults, and can be called from any context (including interrupts).
It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
the tasks can be preempted and, when they are scheduled to run again, the
kernel virtual addresses are restored and still valid.
Therefore, replace kmap() with kmap_local_page() in gfs2_unstuffer_page().
Suggested-by: Fabio M. De Francesco
Signed-off-by: Deepak R Varma
---
Changes in v3:
- Patch included in the patch series
Changes in v2:
- None
fs/gfs2/bmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 8d611fbcf0bd..6b850e2ba5c8 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -58,12 +58,12 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip,
struct buffer_head *dibh,
struct inode *inode = >i_inode;
if (!PageUptodate(page)) {
- void *kaddr = kmap(page);
+ void *kaddr = kmap_local_page(page);
u64 dsize = i_size_read(inode);
memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
- kunmap(page);
+ kunmap_local(kaddr);
SetPageUptodate(page);
}
--
2.34.1
[Cluster-devel] [PATCH v3 2/6] gfs2: Replace kmap_atomic()+memcpy by memcpy_from_page()
kmap_atomic() is deprecated in favor of kmap_local_{folio,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_internal_read() does not depend on the above-mentioned side effects.
Further, memcpy_{from,to}_page() wrappers combine the
{kmap, unmap}_local_page() blocks when they are intended exclusively to
copy contents from/to the temporary mapped page. So, replace the
kmap_atomic()/kunmap_automic() block by the memcpy_from_page() API call.
This change allows to tidy-up code and also eliminate unused variable p.
Suggested-by: Fabio M. De Francesco
Signed-off-by: Deepak R Varma
---
Changes in v3:
- Split as a separate patch for conversion in gfs2_internal_read()
- Use memcpy_from_page() as suggested by Fabio and Andreas G
- Included split version in patch set
Changes in v2:
- Update patch description to correct the replacement function name
from kmap_local_folio to kmap_local _page
fs/gfs2/aops.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 3eac4f2f5c27..f47fed657763 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -489,7 +489,6 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf,
loff_t *pos,
unsigned copied = 0;
unsigned amt;
struct page *page;
- void *p;
do {
page = read_cache_page(mapping, index, gfs2_read_folio, NULL);
@@ -498,12 +497,12 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf,
loff_t *pos,
continue;
return PTR_ERR(page);
}
- p = kmap_atomic(page);
- amt = size - copied;
if (offset + size > PAGE_SIZE)
amt = PAGE_SIZE - offset;
- memcpy(buf + copied, p + offset, amt);
- kunmap_atomic(p);
+ else
+ amt = size - copied;
+
+ memcpy_from_page(buf, page, offset, amt);
put_page(page);
copied += amt;
index++;
--
2.34.1
[Cluster-devel] [PATCH v3 1/6] gfs2: Replace kmap_atomic() by kmap_local_page() in stuffed_readpage
kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
Therefore, replace kmap_atomic() with kmap_local_page() in
stuffed_readpage().
kmap_atomic() disables page-faults and preemption (the latter only for
!PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
stuffed_readpage() 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
Signed-off-by: Deepak R Varma
---
Changes in v3:
- split into 2 patches
- included in the patch set. Was sent as standalone patch previously
Changes in v2:
- Update patch description to correct the replacement function name from
kmap_local_page to kmap_local_folio
fs/gfs2/aops.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 3b41542d6697..3eac4f2f5c27 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -432,10 +432,10 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct
page *page)
if (error)
return error;
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_page(page);
memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
flush_dcache_page(page);
brelse(dibh);
SetPageUptodate(page);
--
2.34.1
[Cluster-devel] [PATCH v3 0/6] gfs2: kmap{_atomic} conversion to kmap_local_{page/folio}
This patch series proposes to replace the kmap/kmap_atomic implementation to the
preferred kmap_local_* APIs.
The code blocks for this module where kmap/kmap_atomic calls are implemented do
not appear to depend on disabling page-faults or preemption. Hence such code
blocks are safe for converting to improved kmap_local_{page,folio} APIs.
Note: The proposed patches are build tested only.
Initially, only a single patch was sent and now being converted into a patch
series including the other files/functions of this module. Hence all patches,
that are included for the first time in this series are also marked as v3.
Changes in v3:
- Patch set introduced to include all gfs2 kmap conversions
- Patches 3/6 through 6/6 are included to build the series
- Initial stand-alone patch split into 2 patches [1/6 and 2/6]
Changes in v2:
- 3/6 to 6/6: None.
- 1/6 + 2/6: Correct patch description for the replacement function name from
kmap_local_folio to kmap_local_page
Deepak R Varma (6):
gfs2: Replace kmap_atomic() by kmap_local_page() in stuffed_readpage
gfs2: Replace kmap_atomic()+memcpy by memcpy_from_page()
gfs2: Replace kmap() by kmap_local_page() in gfs2_unstuffer_page
gfs2: Replace kmap_atomic() by kmap_local_page() in lops.c
gfs2: Replace kmap() by kmap_local_page() in gfs2_read_super
gfs2: Replace kmap_atomic() by kmap_local_page() in
gfs2_write_buf_to_page
fs/gfs2/aops.c | 13 ++---
fs/gfs2/bmap.c | 4 ++--
fs/gfs2/lops.c | 12 ++--
fs/gfs2/ops_fstype.c | 4 ++--
fs/gfs2/quota.c | 4 ++--
5 files changed, 18 insertions(+), 19 deletions(-)
--
2.34.1
Re: [Cluster-devel] [PATCH] gfs2: Replace deprecated kmap_atomic() by kmap_local_page()
On Tue, Jun 27, 2023 at 01:11:46PM +0200, Fabio M. De Francesco wrote:
> On domenica 25 giugno 2023 21:23:21 CEST Deepak R Varma wrote:
> > kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
> >
> > }
> > - p = kmap_atomic(page);
> > + p = kmap_local_page(page);
> > amt = size - copied;
> > if (offset + size > PAGE_SIZE)
> > amt = PAGE_SIZE - offset;
> > memcpy(buf + copied, p + offset, amt);
>
> How about using memcpy_from_page()?
We can do that. I will include that in v3.
Deepak.
>
> Fabio
>
> > - kunmap_atomic(p);
> > + kunmap_local(p);
> > put_page(page);
> > copied += amt;
> > index++;
> > --
> > 2.34.1
>
>
>
>
Re: [Cluster-devel] [PATCH v2] gfs2: Replace deprecated kmap_atomic() by kmap_local_page()
On Tue, Jun 27, 2023 at 03:45:20PM +0200, Andreas Gruenbacher wrote:
> On Mon, Jun 26, 2023 at 8:51 AM Deepak R Varma wrote:
> > kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
>
> I'll apply this, convert the remaining instances of kmap_atomic(), and
> switch to memcpy_{from,to}_page() where appropriate.
Hello Andreas,
Thank you for your review. I am working on the other conversion opportunities
for this module and will send those in shortly.
Regards,
Deepak.
>
> Thanks,
> Andreas
>
> > Therefore, replace kmap_atomic() with kmap_local_page() in
> > gfs2_internal_read() and stuffed_readpage().
> >
> > kmap_atomic() disables page-faults and preemption (the latter only for
> > !PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
> > gfs2_internal_read() and stuffed_readpage() 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()).
> >
> > Signed-off-by: Deepak R Varma
> > ---
> > Note: The Patch is build tested only. I will be happy to run recommended
> > testing
> > with some guidance if required.
> >
> > Changes in v2:
> >- Update patch description to correct the replacement function name from
> > kmap_local_folio to kmap_local _page
> >
> >
> > fs/gfs2/aops.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> > index 3b41542d6697..7bd92054d353 100644
> > --- a/fs/gfs2/aops.c
> > +++ b/fs/gfs2/aops.c
> > @@ -432,10 +432,10 @@ static int stuffed_readpage(struct gfs2_inode *ip,
> > struct page *page)
> > if (error)
> > return error;
> >
> > - kaddr = kmap_atomic(page);
> > + kaddr = kmap_local_page(page);
> > memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
> > memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
> > - kunmap_atomic(kaddr);
> > + kunmap_local(kaddr);
> > flush_dcache_page(page);
> > brelse(dibh);
> > SetPageUptodate(page);
> > @@ -498,12 +498,12 @@ int gfs2_internal_read(struct gfs2_inode *ip, char
> > *buf, loff_t *pos,
> > continue;
> > return PTR_ERR(page);
> > }
> > - p = kmap_atomic(page);
> > + p = kmap_local_page(page);
> > amt = size - copied;
> > if (offset + size > PAGE_SIZE)
> > amt = PAGE_SIZE - offset;
> > memcpy(buf + copied, p + offset, amt);
> > - kunmap_atomic(p);
> > + kunmap_local(p);
> > put_page(page);
> > copied += amt;
> > index++;
> > --
> > 2.34.1
> >
> >
> >
>
[Cluster-devel] [PATCH v2] gfs2: Replace deprecated kmap_atomic() by kmap_local_page()
kmap_atomic() is deprecated in favor of kmap_local_{folio,page}().
Therefore, replace kmap_atomic() with kmap_local_page() in
gfs2_internal_read() and stuffed_readpage().
kmap_atomic() disables page-faults and preemption (the latter only for
!PREEMPT_RT kernels), However, the code within the mapping/un-mapping in
gfs2_internal_read() and stuffed_readpage() 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()).
Signed-off-by: Deepak R Varma
---
Note: The Patch is build tested only. I will be happy to run recommended testing
with some guidance if required.
Changes in v2:
- Update patch description to correct the replacement function name from
kmap_local_folio to kmap_local _page
fs/gfs2/aops.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 3b41542d6697..7bd92054d353 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -432,10 +432,10 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct
page *page)
if (error)
return error;
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_page(page);
memcpy(kaddr, dibh->b_data + sizeof(struct gfs2_dinode), dsize);
memset(kaddr + dsize, 0, PAGE_SIZE - dsize);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
flush_dcache_page(page);
brelse(dibh);
SetPageUptodate(page);
@@ -498,12 +498,12 @@ int gfs2_internal_read(struct gfs2_inode *ip, char *buf,
loff_t *pos,
continue;
return PTR_ERR(page);
}
- p = kmap_atomic(page);
+ p = kmap_local_page(page);
amt = size - copied;
if (offset + size > PAGE_SIZE)
amt = PAGE_SIZE - offset;
memcpy(buf + copied, p + offset, amt);
- kunmap_atomic(p);
+ kunmap_local(p);
put_page(page);
copied += amt;
index++;
--
2.34.1
