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 <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Signed-off-by: Fabio M. De Francesco <[email protected]>
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.
On Mon, May 16, 2022 at 03:55:54PM +0100, 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 <[email protected]>
> > Reviewed-by: Ira Weiny <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
>
> 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.
I'm not against trying to test. But did you see a place which might break
the kmap_local() rules?
Ira
On Mon, May 16, 2022 at 03:55:54PM +0100, 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 <[email protected]>
> > Reviewed-by: Ira Weiny <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
>
> 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.
Do you know of any real users of UFS?
Fabio and I have been looking into how to test this and it seems like UFS
support has been dropped in my system. For example, there is no mkfs.ufs in my
fc35 system.
Searching google, I see that mkfs.ufs turns up a couple of Oracle documents.
And some other links mention something called newfs which I've never heard of.
The patches follow the same pattern which was added to ext2 a while back and
have not caused an issue. So I'm pretty confident they will be ok.
However, if it is critical that these be tested I think Fabio will have to hold
off on these patches for now as there are plenty of other kmap() call sites
which are more important to be fixed.
Ira
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 <[email protected]>
> > Reviewed-by: Ira Weiny <[email protected]>
> > Signed-off-by: Fabio M. De Francesco <[email protected]>
>
> 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
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 <[email protected]>
> > > Reviewed-by: Ira Weiny <[email protected]>
> > > Signed-off-by: Fabio M. De Francesco <[email protected]>
> >
> > 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