Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To:     Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>,
        Nick Terrell <terrelln@fb.com>,
        Chris Down <chris@chrisdown.name>,
        Filipe Manana <fdmanana@suse.com>,
        Nikolay Borisov <nborisov@suse.com>,
        Gabriel Niebler <gniebler@suse.com>,
        linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        Qu Wenruo <wqu@suse.com>
Cc:     Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH] btrfs: Convert zlib_compress_pages() to use kmap_local_page()
Date:   Sun, 26 Jun 2022 13:12:31 +0200
Message-ID: <2656130.mvXUDI8C0e@opensuse>
In-Reply-To: <f65fd4fb-675a-3f9e-accb-77db9ad7d551@suse.com>
References: <20220618092752.25153-1-fmdefrancesco@gmail.com> <3110135.5fSG56mABF@opensuse> <f65fd4fb-675a-3f9e-accb-77db9ad7d551@suse.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

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 <wqu@suse.com>
> >> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> >> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> >> ---
> >>
> >> 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