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. -Eric > Thanks, > Ryusuke Konishi