2022-06-18 09:29:53

by Fabio M. De Francesco

[permalink] [raw]
Subject: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()

The use of kmap() is being deprecated in favor of kmap_local_page(). With
kmap_local_page(), the mapping is per thread, CPU local and not globally
visible.

Therefore, use kmap_local_page() / kunmap_local() in zlib_compress_pages()
because in this function the mappings are per thread and are not visible
in other contexts.

Tested with xfstests on QEMU + KVM 32-bit VM with 4GB of RAM and
HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".

Cc: Qu Wenruo <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---

This patch builds only on top of
"[PATCH] btrfs: zlib: refactor how we prepare the input buffer" by Qu Wenruo".
https://lore.kernel.org/linux-btrfs/d0bfc791b5509df7b9ad44e41ada197d1b3149b3.1655519730.git.wqu@suse.com/

fs/btrfs/zlib.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 966e17cea981..4496dd30bd71 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -160,7 +160,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
pages[0] = out_page;
nr_pages = 1;

@@ -198,9 +198,9 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
* the stream end if required
*/
if (workspace->strm.avail_out == 0) {
- kunmap(out_page);
+ kunmap_local(cpage_out);
if (nr_pages == nr_dest_pages) {
- out_page = NULL;
+ cpage_out = NULL;
ret = -E2BIG;
goto out;
}
@@ -209,7 +209,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
pages[nr_pages] = out_page;
nr_pages++;
workspace->strm.avail_out = PAGE_SIZE;
@@ -236,9 +236,9 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
goto out;
} else if (workspace->strm.avail_out == 0) {
/* get another page for the stream end */
- kunmap(out_page);
+ kunmap_local(cpage_out);
if (nr_pages == nr_dest_pages) {
- out_page = NULL;
+ cpage_out = NULL;
ret = -E2BIG;
goto out;
}
@@ -247,7 +247,7 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
ret = -ENOMEM;
goto out;
}
- cpage_out = kmap(out_page);
+ cpage_out = kmap_local_page(out_page);
pages[nr_pages] = out_page;
nr_pages++;
workspace->strm.avail_out = PAGE_SIZE;
@@ -266,8 +266,8 @@ int zlib_compress_pages(struct list_head *ws, struct address_space *mapping,
*total_in = workspace->strm.total_in;
out:
*out_pages = nr_pages;
- if (out_page)
- kunmap(out_page);
+ if (cpage_out)
+ kunmap_local(cpage_out);
return ret;
}

--
2.36.1


2022-06-25 14:43:28

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()

On sabato 18 giugno 2022 11:27:52 CEST Fabio M. De Francesco wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page(). With
> kmap_local_page(), the mapping is per thread, CPU local and not globally
> visible.
>
> Therefore, use kmap_local_page() / kunmap_local() in
zlib_compress_pages()
> because in this function the mappings are per thread and are not visible
> in other contexts.
>
> Tested with xfstests on QEMU + KVM 32-bit VM with 4GB of RAM and
> HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".
>
> Cc: Qu Wenruo <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
> ---
>
> This patch builds only on top of
> "[PATCH] btrfs: zlib: refactor how we prepare the input buffer" by Qu
Wenruo".
> https://lore.kernel.org/linux-btrfs/
d0bfc791b5509df7b9ad44e41ada197d1b3149b3.1655519730.git.wqu@suse.com/
>

I've seen that Qu sent a v2 of the above patch. However David thinks that
it is better not to map pages allocated in zlib.c for output (out_page)
since they cannot come from highmem because of "alloc_page(GFP_NOFS);".

@David:

I suppose that, since it builds _only_ on top of the refactor submitted by
Qu, I'll have to wait and see what you decide.

If you don't want kmap_local_page() and prefer using page_address() on
"out_page", please drop this patch and let me know, so that I can send a
new patch which will be in accordance to your preference.

Thanks,

Fabio


2022-06-25 23:21:46

by Qu Wenruo

[permalink] [raw]
Subject: Re: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()



On 2022/6/25 22:41, Fabio M. De Francesco wrote:
> On sabato 18 giugno 2022 11:27:52 CEST Fabio M. De Francesco wrote:
>> The use of kmap() is being deprecated in favor of kmap_local_page(). With
>> kmap_local_page(), the mapping is per thread, CPU local and not globally
>> visible.
>>
>> Therefore, use kmap_local_page() / kunmap_local() in
> zlib_compress_pages()
>> because in this function the mappings are per thread and are not visible
>> in other contexts.
>>
>> Tested with xfstests on QEMU + KVM 32-bit VM with 4GB of RAM and
>> HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".
>>
>> Cc: Qu Wenruo <[email protected]>
>> Suggested-by: Ira Weiny <[email protected]>
>> Signed-off-by: Fabio M. De Francesco <[email protected]>
>> ---
>>
>> This patch builds only on top of
>> "[PATCH] btrfs: zlib: refactor how we prepare the input buffer" by Qu
> Wenruo".
>> https://lore.kernel.org/linux-btrfs/
> d0bfc791b5509df7b9ad44e41ada197d1b3149b3.1655519730.git.wqu@suse.com/
>>
>
> I've seen that Qu sent a v2 of the above patch. However David thinks that
> it is better not to map pages allocated in zlib.c for output (out_page)
> since they cannot come from highmem because of "alloc_page(GFP_NOFS);".
>
> @David:
>
> I suppose that, since it builds _only_ on top of the refactor submitted by
> Qu, I'll have to wait and see what you decide.
>
> If you don't want kmap_local_page() and prefer using page_address() on
> "out_page", please drop this patch and let me know, so that I can send a
> new patch which will be in accordance to your preference.

And that would also make the convert much easier for kmap_local_page()
of input pages.

I'll hold the refactor patch after all the kmap code is converted.

Thanks,
Qu

>
> Thanks,
>
> Fabio
>
>

2022-06-26 11:49:04

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()

On domenica 26 giugno 2022 01:03:55 CEST Qu Wenruo wrote:
>
> On 2022/6/25 22:41, Fabio M. De Francesco wrote:
> > On sabato 18 giugno 2022 11:27:52 CEST Fabio M. De Francesco wrote:
> >> The use of kmap() is being deprecated in favor of kmap_local_page().
> >> With kmap_local_page(), the mapping is per thread, CPU local and not
> >> globally visible.
> >>
> >> Therefore, use kmap_local_page() / kunmap_local() in
> >> zlib_compress_pages() because in this function the mappings are per
> >> thread and are not visible in other contexts.
> >>
> >> Tested with xfstests on QEMU + KVM 32-bit VM with 4GB of RAM and
> >> HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".
> >>
> >> Cc: Qu Wenruo <[email protected]>
> >> Suggested-by: Ira Weiny <[email protected]>
> >> Signed-off-by: Fabio M. De Francesco <[email protected]>
> >> ---
> >>
> >> This patch builds only on top of
> >> "[PATCH] btrfs: zlib: refactor how we prepare the input buffer" by Qu
> >> Wenruo".
> >> https://lore.kernel.org/linux-btrfs/
d0bfc791b5509df7b9ad44e41ada197d1b3149b3.1655519730.git.wqu@suse.com/
> >>
> >
> > I've seen that Qu sent a v2 of the above patch. However David thinks
> > that it is better not to map pages allocated in zlib.c for output
> > (out_page) since they cannot come from highmem because of
> > "alloc_page(GFP_NOFS);".
> >
> > @David:
> >
> > I suppose that, since it builds _only_ on top of the refactor submitted
> > by
> > Qu, I'll have to wait and see what you decide.
> >
> > If you don't want kmap_local_page() and prefer using page_address() on
> > "out_page", please drop this patch and let me know, so that I can send
a
> > new patch which will be in accordance to your preference.
>
> And that would also make the convert much easier for kmap_local_page()
> of input pages.
>
> I'll hold the refactor patch after all the kmap code is converted.

Thanks Qu,

I have already made a patch to zlib_compress_pages() in accordance to what
David asked for, but I cannot compile and test it.

With last week update of Tumbleweed, openSUSE dropped GCC-10 for x86_32, so
we've been left only with GCC-7. With GCC-7 I cannot any longer build
5.19.rc3 because it fails somewhere in drm/i915 (where code has not changed
since April 2022).

I suppose it's just a mistake which they will fix within few days.

I'm pretty sure that my patch works properly, however I'm not comfortable
to submit it with no successful build and tests.

Again thanks,

Fabio



2022-06-27 16:53:18

by Fabio M. De Francesco

[permalink] [raw]
Subject: Re: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()

On domenica 26 giugno 2022 01:03:55 CEST Qu Wenruo wrote:
>
> On 2022/6/25 22:41, Fabio M. De Francesco wrote:
> > On sabato 18 giugno 2022 11:27:52 CEST Fabio M. De Francesco wrote:
> >> The use of kmap() is being deprecated in favor of kmap_local_page().
With
> >> kmap_local_page(), the mapping is per thread, CPU local and not
globally
> >> visible.
> >>
> >> Therefore, use kmap_local_page() / kunmap_local() in
> > zlib_compress_pages()
> >> because in this function the mappings are per thread and are not
visible
> >> in other contexts.
> >>
> >> Tested with xfstests on QEMU + KVM 32-bit VM with 4GB of RAM and
> >> HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".
> >>
> >> Cc: Qu Wenruo <[email protected]>
> >> Suggested-by: Ira Weiny <[email protected]>
> >> Signed-off-by: Fabio M. De Francesco <[email protected]>
> >> ---
> >>
> >> This patch builds only on top of
> >> "[PATCH] btrfs: zlib: refactor how we prepare the input buffer" by Qu
> > Wenruo".
> >> https://lore.kernel.org/linux-btrfs/
> > d0bfc791b5509df7b9ad44e41ada197d1b3149b3.1655519730.git.wqu@suse.com/
> >>
> >
> > I've seen that Qu sent a v2 of the above patch. However David thinks
that
> > it is better not to map pages allocated in zlib.c for output (out_page)
> > since they cannot come from highmem because of "alloc_page(GFP_NOFS);".
> >
> > @David:
> >
> > I suppose that, since it builds _only_ on top of the refactor submitted
by
> > Qu, I'll have to wait and see what you decide.
> >
> > If you don't want kmap_local_page() and prefer using page_address() on
> > "out_page", please drop this patch and let me know, so that I can send
a
> > new patch which will be in accordance to your preference.
>
> And that would also make the convert much easier for kmap_local_page()
> of input pages.
>
> I'll hold the refactor patch after all the kmap code is converted.
>
> Thanks,
> Qu

Thanks for holding the refactor patch for the time I needed.

Now zlib.c is converted to kmap_local_page() and tested:
https://lore.kernel.org/lkml/[email protected]/

Again thanks,

Fabio