linux-kernel - Re: [PATCH v3] fs/ufs: Replace kmap() with kmap_local

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4784407.31r3eYUQgx@opensuse>
Date:   Wed, 03 Aug 2022 21:04:32 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Evgeniy Dushistov <dushistov@...l.ru>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Ira Weiny <ira.weiny@...el.com>, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] fs/ufs: Replace kmap() with kmap_local_page()

On martedì 2 agosto 2022 09:06:26 CEST Fabio M. De Francesco wrote:
> 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@...el.com>
> > > Reviewed-by: Ira Weiny <ira.weiny@...el.com>
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.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
> 
For the sake of completeness I'd like to add something that I forgot to 
mention in the last email...

The only reference to creating a ufs file system I can find is many years 
old and shows using 'newfs' which seems to be a precursor to mkfs.[1] mkfs 
does not seem to support ufs.[2][3].

This is why I'm not sure how to begin testing a ufs file system.

[1] https://docs.oracle.com/cd/E19683-01/806-4073/6jd67r9it/index.html
[2] https://linux.die.net/man/8/mkfs
[3] https://linux.die.net/man/5/fs

Thanks,

Fabio