lkml.org 
[lkml]   [2022]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH v2 2/3] btrfs: Use kmap_local_page() on "out_page" in zlib_compress_pages()
Date
On venerdì 17 giugno 2022 14:54:22 CEST Qu Wenruo wrote:
>
> On 2022/6/17 20:05, 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() for "out_page" 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 bits VM with 4GB of RAM and
> > HIGHMEM64G enabled. This patch passes 26/26 tests of group "compress".
> >
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>
> Looks good to me.
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>

Again, thanks.

> The change is just to use @cpage_out to indicate if it's mapped (NULL =
> not mapped).
>

Most of the conversions are quite easy, I would say "mechanical".
As you already noted in 3/3, there are cases where it's not that simple :-(

> Just a small nit inlined below.
>
> > ---
> > fs/btrfs/zlib.c | 20 +++++++++++---------
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
> > index 770c4c6bbaef..c7c69ce4a1a9 100644
> > --- a/fs/btrfs/zlib.c
> > +++ b/fs/btrfs/zlib.c
> > @@ -97,8 +97,8 @@ int zlib_compress_pages(struct list_head *ws, struct
address_space *mapping,
> > {
> > struct workspace *workspace = list_entry(ws, struct workspace,
list);
> > int ret;
> > - char *data_in;
> > - char *cpage_out;
> > + char *data_in = NULL;
>
> I didn't see any diff touching @data_in, any idea why it's initialized
> to NULL?
>

I suppose it's a relic of RFC v1 that I overlooked when I split it into
three patches. I will remember to avoid initialization in the "real" patch.

Fabio

>
> Thanks,
> Qu
>
> > + char *cpage_out = NULL;
> > int nr_pages = 0;
> > struct page *in_page = NULL;
> > struct page *out_page = NULL;
> > @@ -126,7 +126,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;
> >
> > @@ -196,7 +196,8 @@ 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);
> > + cpage_out = NULL;
> > if (nr_pages == nr_dest_pages) {
> > out_page = NULL;
> > ret = -E2BIG;
> > @@ -207,7 +208,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;
> > @@ -234,7 +235,8 @@ 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);
> > + cpage_out = NULL;
> > if (nr_pages == nr_dest_pages) {
> > out_page = NULL;
> > ret = -E2BIG;
> > @@ -245,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;
> > @@ -264,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);
> >
> > if (in_page) {
> > kunmap(in_page);
>




\
 
 \ /
  Last update: 2022-06-17 19:48    [W:0.509 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site