tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: [PATCH] Support INCOMPAT_64BIT on ext4



I noticed that the two cgload calls are different:

  e2fs_cgload(bp->b_data,
                    &fs->e2fs_gd[i * fs->e2fs_bsize / sizeof(struct ext2_gd)],
                    fs->e2fs_bsize, 1 << fs->e2fs_group_desc_shift);
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
int32_t sh = m_fs->e2fs_bsize >> m_fs->e2fs_group_desc_shift;
e2fs_cgload(bp->b_data, &m_fs->e2fs_gd[i * sh],
                                                                    ^^^^^
                    m_fs->e2fs_bsize, m_fs->e2fs_group_desc_shift); 

e2fs_cgsave(&fs->e2fs_gd[
                    i * fs->e2fs_bsize / sizeof(struct ext2_gd)],
                    bp->b_data, fs->e2fs_bsize, fs->e2fs_group_desc_shift);

shouldn't we consistently index as:
&fs->e2fs_gd[i * fs->e2fs_bsize / sizeof(struct ext2_gd)]
and pass the shift as:
fs->e2fs_group_desc_shift

???

Thanks,

christos

On Aug 26, 2023, at 7:43 PM, Vladimir 'phcoder' Serbinenko <phcoder%gmail.com@localhost> wrote:



Le dim. 27 août 2023, 00:48, Christos Zoulas <christos%zoulas.com@localhost> a écrit :

I took care of it. One I think was mine, one was yours  :-)



Thank you a lot. I was diagnosing it but you were faster. I'll see what I can do for userspace to remove old cgload/cgsave


christos


On Aug 26, 2023, at 8:52 AM, Christos Zoulas <christos%zoulas.com@localhost> wrote:


It could also be my fault because I refactored the code a bit. One of the things that I changed

that looked like a bug to me was adding a cast to e2fs_cgload():

memset((char *)optr + 32, 0, sizeof(*optr) - 32);


since optr is struct ext2_gd *...


christos



On Aug 26, 2023, at 7:46 AM, Vladimir 'phcoder' Serbinenko <phcoder%gmail.com@localhost> wrote:


I'll have a look today or tomorrow 


Le sam. 26 août 2023, 12:06, Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost> a écrit :

> Date: Wed, 23 Aug 2023 04:54:34 +0200
> From: "Vladimir 'phcoder' Serbinenko" <phcoder%gmail.com@localhost>
> 
> This patch adds support for incompat_64bit on ext4 filesystem. This feature
> is enabled by default on new filesystems on Ubuntu and probably other
> distros

Cool, thanks!  christos@ committed this.

It looks like it may have some issues, though -- a lot of ext2fs tests
are failing now.  Can you please take a look at the failures?

https://releng.netbsd.org/b5reports/i386/commits-2023.08.html#build-2023.08.26.05.47.53

   Termination reason

   FAILED: create file: No space left on device

   Standard output stream

   [   1.0000000] entropy: ready
   [   1.0200050] uid 0 on /mnt: out of inodes

FYI, if you're running a new enough kernel already, you can run the
tests yourself by doing a distribution build and then doing

# chroot /path/to/objdir/destdir.amd64
(chroot)# (cd /dev && sh MAKEDEV all)
(chroot)# mount -t ptyfs ptyfs /dev/pts
(chroot)# mount -t tmpfs tmpfs /tmp
(chroot)# cd /usr/tests/fs/vfs
(chroot)# atf-run | atf-report

You can also do build.sh release, install into a VM, and run the tests
the same way -- cd /usr/tests/fs/vfs && atf-run | atf-report.

This also broke the build of bootloaders and the newfs_ext2fs userland
tool.  I put in a stop-gap measure to restore the old definitions of
e2fs_cgload/cgsave outside the kernel, just to unbreak the build, but
it might be appropriate to make the new definitions available to
bootloaders and newfs_ext2fs too.

(I haven't looked into technical details to see whether it is
appropriate.  Probably newfs_ext2s should avoid creating file systems
this new feature for a while, but maybe have an option to do it so we
can exercise the code paths in tests.)


<sanitizer.log>














<sanitizer.log>

Attachment: signature.asc
Description: Message signed with OpenPGP



Home | Main Index | Thread Index | Old Index