Re: [PATCH v3] fs/ufs: Replace kmap() with kmap_local_page()

["Fabio M. De Francesco" <fmdefrancesco@gmail.com>]

On lunedì 16 maggio 2022 16:55:54 CEST Matthew Wilcox wrote:
> On Mon, May 16, 2022 at 12:19:25PM +0200, 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.
> > 
> > The usage of kmap_local_page() in fs/ufs is pre-thread, therefore 
replace
> > kmap() / kunmap() calls with kmap_local_page() / kunmap_local().
> > 
> > kunmap_local() requires the mapping address, so return that address 
from
> > ufs_get_page() to be used in ufs_put_page().
> > 
> > These changes are essentially ported from fs/ext2 and are largely based 
on
> > commit 782b76d7abdf ("fs/ext2: Replace kmap() with kmap_local_page()").
> > 
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> 
> Have you done more than compile-tested this?  I'd like to know that it's
> been tested on a machine with HIGHMEM enabled (in a VM, presumably).
> UFS doesn't get a lot of testing, and it'd be annoying to put out a
> patch that breaks the kmap_local() rules.
> 
As said in another message of this thread, these changes have only been 
compile-tested. I can't see anything which may break the rules about using 
local mappings properly.

I'm working on converting all kmap() call sites I can do across the whole 
kernel to kmap_local_page(). Practically all of those conversions have 
already been reviewed / acked, and many of them have already been taken by 
their respective maintainers. Others are still too recent.

Most of those patches have been properly tested on a QEMU/KVM x86_32 VM, 
4GB to 6GB RAM, booting kernels with HIGHMEM64GB enabled.

Instead, despite this submission is very old, I haven't yet been able to 
figure out how to test these changes. I really don't know how I can create 
and test a UFS filesystem.

Can you please help somewhat with hints about how to test this patch or 
with testing it yourself? I'm thinking of this option because I suppose 
that you may have access to a Solaris system (if I recall correctly, UFS is 
the default filesystem of that OS. Isn't it?).

I'm sorry to bother you with this issue, however I'd appreciate any help 
you may provide. I'd hate to see all patches applied but one :-) 

Thanks,

Fabio