On Fri, Apr 5, 2024 at 5:35 AM Eric Sandeen wrote: > > On 4/4/24 3:11 PM, Ryusuke Konishi wrote: > > On Thu, Apr 4, 2024 at 7:12 AM Eric Sandeen wrote: > >> > >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> > >> --- > >> > >> Note: This one was relatively more complex than others, so I would > >> appreciate review and testing. Basic checks of mounts, various mount > >> options, and snapshot mounts do seem to work. I may well have missed > >> something though, as I am not very familiar with nilfs. > >> > >> You may want to look closely at the handling of case Opt_err: which > >> no longer uses nilfs_write_opt() and open-codes the flag change, so > >> that I can use the enum. If you'd prefer to make 3 independent > >> Opt_err_XXXZ cases, that would be possible. > >> > >> If any of the other changes here are unclear, or problematic, please > >> let me know. > >> > >> Thanks! > >> -Eric > > > > Hi Eric, > > > > Thank you! This is one of the modernizations that I thought I had to > > do with nilfs2. > > > > I'm planning on doing a full review later, but when I ran a mount > > pattern test, the kernel restarted without any messages (probably > > caused a panic), so I'll give you some quick feedback. > > > > The mount pattern that caused the kernel to restart was a simultaneous > > mount of the current tree and a snapshot, which occurred when the > > snapshot was mounted and then the current tree was mounted. Something > > like below: > > > > $ sudo losetup /dev/loop0 ./nilfs.iso > > $ sudo mount -t nilfs2 -o ro,cp=38866 /dev/loop0 /mnt/snapshot > > $ sudo mount -t nilfs2 /dev/loop0 /mnt/test > > --> panic > > > > Here, 38866 is the snapshot number that can be created with the > > nilfs-utils "mkcp -s" command or "chcp" command, and the number can be > > checked with "lscp -s". > > > > I have placed the mount test script I used in the following location: > > > > https://github.com/konis/nilfs-test-tools/blob/main/test-nilfs-mount.sh > > > > The panic occurred in test #17 of that script. > > > > I'll also try to track what's going on. > > Thanks, I'll look - I was hoping/expecting that you had better tests for > mount options than I did! ;) > > Feel free to debug if you like, but it must be a bug in my patch so > I'll take ownership of trying to track down the problem and get it to > pass your test script. Got it! So I'll try to understand the patch first. This test script focuses on reproducing NILFS-specific mount sequences (such as mounting a snapshot and current tree simultaneously) and checking the state of user space such as the GC process and utab. And, is not exhaustive for mount options. Looking at the patch, if I come up with test patterns that would be better to add, I will enhance the test script. Thanks, Ryusuke Konishi > > -Eric > > > Thanks, > > Ryusuke Konishi >
