In general b_lblkno is for the use of the client of the buffer cache to use for whatever purpose they want. The b_blkno is what the underlying hardware is going to use to locate the requested data. The filesystem generally uses its BMAP routine to convert the b_lblkno to the appropriate b_blkno.

In D38789#883134, @fbsd_paepcke.de wrote:

[...] It would be possible to add another flag to the existing um_flags word with the semantic that the filesystem can be mounted only as read-only (UM_READONLY). Of course it would still be possible for the superuser to clear this flag but it takes slightly more effort.

Yes, that is a great approach!
I will look into it / make a proposal.

In a second step, we could guard the UM_READONLY flag via the
existing FFS superblock checksum logic in way that fsck will detect
and refuse to auto-repair a manual flag clean attempt.

Thank you!

Relevant parts of these changes were incorporated in commit 772430dd6795585 as discussed in PR 267654 and review https://reviews.freebsd.org/D41724.

This does fix the problem as described in the summary.

If I understand your change, you are coming up with a different synthetic inode value to ensure that it will be unique. Presumably this value is externally visible as the "st_ino" in the stat structure and the "d_fileno" in the dirent structure. If so, this value will be different after this change so any program that has saved it somewhere could get confused. Not sure if this is really an issue though.

Non-obvious bug, but once figured out the fix is simple and clearly correct.

I would be happy to review a subsequent patch to make i_nlink a 16-bit unsigned int and move/adapt the KASSERT() checking for underflow.

Change in-memory i_nlink to a signed 32-bit value so that can check that it has not gone negative before assigning it to the on-disk unsigned 16-bit di_nlink field.

In D42767#976082, @mckusick wrote:

In D42767#975487, @kib wrote:

ffs_softdep.c has several lines of kind

sys/ufs/ffs/ffs_softdep.c:    KASSERT(ip->i_nlink >= 0, ("handle_workitem_remove: file ino "

which no longer make sense.

There are less critical comparisons like ip->i_nlink <= 0 at least in ufs_inode.c and ufs_vnops.c which could be cleaned up.

Actually i_nlink is a signed 32-bit value where the nlink count is maintained while the inode is in memory. So it can in fact be checked for negative values which of course should never be copied to the 16-bit unsigned value that is maintained on the disk.

In D42767#975487, @kib wrote:

ffs_softdep.c has several lines of kind

sys/ufs/ffs/ffs_softdep.c:    KASSERT(ip->i_nlink >= 0, ("handle_workitem_remove: file ino "

which no longer make sense.

There are less critical comparisons like ip->i_nlink <= 0 at least in ufs_inode.c and ufs_vnops.c which could be cleaned up.

Per kib suggestions:

Comments from kib.

Sorry, I thought I had approved this and missed your (useful) update.

Updates reflecting comments from kib.

I do not know which if any disk controllers are currently affected. It is a problem that comes and goes. Adding the alignment hook allows buffer alignment to be easily adjusted as needed. It also has the performance improvement of not requiring disk drivers to use bounce buffers in cases that they previously needed to do so.

Updates reflecting comments from kib and imp.

This all looks good to go. Thanks for doing it.

Update to reflect suggested changes by kib.

Your checksum test is a good way to verify that everything worked as expected though it is possible that some sort of filesystem corruption snuck in (obviously to both copies). So you might want to add a second check by running 'fsck -p -f' on the both the gunion filesystem and the original filesystem after the gunion has been dissolved. The exit status from fsck should be zero.

I am happy to see a set of gunion tests being added. The set of tests looks complete.

This looks like the correct solution to me. We are at the point where we decide we need to do a forcible unmount and realize that we are about to do it on the root filesystem.

For system calls you do need annotations, you cannot properly understand the purpose of any pointer passed to syscall otherwise. Is it in, out, in/out, or even just an abstract address like mmap/munmap/madvise arguments?

For ioctls, the meaning of the command/arg is quite formalized, at least in BSDs. You are well aware that we encode both sizes and directions for copyin and copyout. After I took some time thinking how to implement what you described, I was surprised by the statement that syscalls are easier than ioctls.

That said, I am quite dislike extending the syscall semantical coverage by adding one-off operations. In the case of special fds, having all ops grouped together (under ioctl umbrella) localizes the interfaces and make them more comprehensive. This is, of course, my opinion, but all my experience supports the claim.