[PATCH] fs/sysv: Don't round down address for kunmap_flush_on_unmap()

[PATCH] fs/sysv: Don't round down address for kunmap_flush_on_unmap()

[Fabio M. De Francesco]

The kernel virtual address passed to kunmap_flush_on_unmap() has no more
any need to be rounded down.

Therefore, delete the rounding down of "page_addr" when passed to
kunmap_local() in dir_put_page().

Don't backport without commit 88d7b12068b9 ("highmem: round down the
address passed to kunmap_flush_on_unmap()").

Cc: Ira Weiny <ira.weiny@intel.com>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
 fs/sysv/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/sysv/dir.c b/fs/sysv/dir.c
index 999bceb99974..e2d26eb78af7 100644
--- a/fs/sysv/dir.c
+++ b/fs/sysv/dir.c
@@ -30,7 +30,7 @@ const struct file_operations sysv_dir_operations = {
 
 inline void dir_put_page(struct page *page, void *page_addr)
 {
-	kunmap_local((void *)((unsigned long)page_addr & PAGE_MASK));
+	kunmap_local(page_addr);
 	put_page(page);
 }
 
-- 
2.39.2

Re: [PATCH] fs/sysv: Don't round down address for kunmap_flush_on_unmap()

[Al Viro]

On Mon, Mar 06, 2023 at 01:51:50PM +0100, Fabio M. De Francesco wrote:
> The kernel virtual address passed to kunmap_flush_on_unmap() has no more
> any need to be rounded down.
> 
> Therefore, delete the rounding down of "page_addr" when passed to
> kunmap_local() in dir_put_page().
> 
> Don't backport without commit 88d7b12068b9 ("highmem: round down the
> address passed to kunmap_flush_on_unmap()").

Applied (#work.misc).  FWIW, I've rebased the ext2 series to -rc1 (and
realized what got Jan confused about ext2_rename() changes).

Re minixfs: it's actually very close to sysv, so much that at one point
I considered merging them - making minixfs one of sysvfs flavours.

Think of it as v7 filesystem with the simpler improvements copied from
FFS.  Cylinder groups and variable-sized directory entries - too
complex for Minix purposes.  Lifting the name length limit from 14 to
30 - sure, why not?  32bit block numbers - eventually made it,
so did 32bit inode numbers (in v3).  

The main advance compared to v7 is the use of bitmaps for block
and inode allocation.  Unlike FFS it's all in one lump, but at least
it's not the "free block list".

For directory contents handling it doesn't matter at all - there minixfs
is really just another sysvfs variant.  Directory is stored the same
way as a regular file would've been, the data in it is an array of
fixed-sized entries (16, 32 or 64 bytes, depending upon the filesystem
version), each consisting of inode number (2 or 4 bytes) + array of
characters representing the name; name shorter than the longest possible
are NUL-terminated.

Anyway, I've slapped together a counterpart of your sysv series,
see #work.minix

Re: [PATCH] fs/sysv: Don't round down address for kunmap_flush_on_unmap()

[Fabio M. De Francesco]

On lunedì 6 marzo 2023 18:27:59 CET Al Viro wrote:
> On Mon, Mar 06, 2023 at 01:51:50PM +0100, Fabio M. De Francesco wrote:
> > The kernel virtual address passed to kunmap_flush_on_unmap() has no more
> > any need to be rounded down.
> > 
> > Therefore, delete the rounding down of "page_addr" when passed to
> > kunmap_local() in dir_put_page().
> > 
> > Don't backport without commit 88d7b12068b9 ("highmem: round down the
> > address passed to kunmap_flush_on_unmap()").
> 
> Applied (#work.misc).

Thanks!

I'm using (again, sorry) this opportunity to remind you that I'd really 
appreciate if you could also set aside some time to look at my patch to fs/
aio.c.

Instead I'm not sure yet who is at the moment responsible for the patches to 
fs/ufs... 

> FWIW, I've rebased the ext2 series to -rc1 (and
> realized what got Jan confused about ext2_rename() changes).

I just git-clone(ed) your "vfs" tree and started with building and testing the  
#work.ext2 branch, without and with your latest commits (from Linux 6.3-rc1 
merge onward).

As said in the thread with the pull request of my fs/sysv related patches I'll 
test with (x)fstests in a QEMU/KVM x86_32 VM, 6GB RAM, running an HIGHMEM64GB 
kernel.

> Re minixfs: it's actually very close to sysv, so much that at one point
> I considered merging them - making minixfs one of sysvfs flavours.

OK, so porting fs/sysv (or fs/ufs) changes to fs/minix should be an easy task.

> Think of it as v7 filesystem with the simpler improvements copied from
> FFS.  Cylinder groups and variable-sized directory entries - too
> complex for Minix purposes.  Lifting the name length limit from 14 to
> 30 - sure, why not?  32bit block numbers - eventually made it,
> so did 32bit inode numbers (in v3).
> 
> The main advance compared to v7 is the use of bitmaps for block
> and inode allocation.  Unlike FFS it's all in one lump, but at least
> it's not the "free block list".
> 
> For directory contents handling it doesn't matter at all - there minixfs
> is really just another sysvfs variant.  Directory is stored the same
> way as a regular file would've been, the data in it is an array of
> fixed-sized entries (16, 32 or 64 bytes, depending upon the filesystem
> version), each consisting of inode number (2 or 4 bytes) + array of
> characters representing the name; name shorter than the longest possible
> are NUL-terminated.
> 
> Anyway, I've slapped together a counterpart of your sysv series,
> see #work.minix

I must admit that I don't yet own enough good knowledge of the details you are 
mentioning above. However, thanks for sharing!

Those insights surely help me to better understand what details to look at 
when trying to tell the differences and commonalities between filesystems.

I'll take a look at #work.minix in the next days (build, test, review), but 
only when I'm done with #work.ext2 (I have little spare time because ATM I'm 
attending some courses and, in the meantime, I'm also volunteering as a co-
mentor with Ira).

Again thanks,

Fabio