With the removal of spacectl_range32 this whole function is redundant.

This is for MNT_SHARED_WRITES(mp) == true, which is the case in ZFS.

Since we are now at FreeBSD 14, this should be in FBSD_1.7 namespace.

(Unrelated: I always wonder why we can't just call future versions e.g. FBSD_15.0; I usually find myself looking at Version.def to confirm)

I think it's worth commenting here that the reason we are CTASSERT'ing the size was because we wanted to make sure that for both 32-bit and 64-bit variants, the struct have the same layout (and thus the compat32 call can just use the same struct); having it in the same block of asserting size_t was confusing for me in the first glance.

Is this intentional? If so, could you please add a comment explaining why we are ignoring ERESTART/EINTR/EWOULDBLOCK? (I think this is different from short read/write and these shouldn't be ignored?)

It was intentional, but yes they shouldn't be ignored as short read/write.

I believe that was already discussed, and now you returned back the same errant behavior. Unix convention is that error return is only acceptable if the file content was not changed at all. If syscall did modified the file, regardless was it completed WRT requested op, or just a partial execution, userspace must know exactly what was done.

I think that this requires translation for compat32, because spacectl_range contains off_t. Look how freebsd32_lseek() handles that, in particular the big- vs. little-endian versions of PAIR32TO64.

I wonder if it make sense to change the fspacectl(2) interface while not too late and split in and out spacectl_range. Basically keep original spacectl_range as is, and copy out optional processed length:

fspacectl(int fd, int cmd, const struct spacectl_range *sr, int flags, off_t *length);

Why don't you use fget_write()? I am not even sure why you check for CAP_PWRITE instead of CAP_WRITE, the later seems to be more suitable.

This was copied from shm_dotruncate_locked(), right? It misses the vm_pager_has_page() case. Basically, the page that you need to partially zero might be not resident but swap contains the user data. So we need to read the page from swap.

Perhaps you should refactor code to instantiate the page from shm_dotruncate_locked() and reuse it. Perhaps the code should take some control which region of the page should be cleared, to be passed to pmap_zero_page_area().

Does it sound better to have copyout spacectl_range instead of copyout length, so that callers may save an fseek(DATA) call to guess where to restart?

But if we can in theory operate in any positions in a file, we should use CAP_PWRITE?

Why do you copyout result only in case of an error? This is esp. not useful for ERESTART/EINTR etc case where you cleared the error.

I suggest to extract introduction of vn_bmap_seekhole_locked() into a separate commit and do it in advance.

This assert should be either repeated in vn_bmap_seekhole_locked(), or moved to that function.

Use bool?

This condition is still not fixed.

BTW, I suggest to remove the error != 0 check at all. Copy *rqsr into *rmsr at the very beginning of kern_fspacectl() if rmsr != NULL, and it should be fine.

You did not fixed this

I still insist on copying out rmsr always, not only on error == 0. For instance, if errno == ENOSPC, you still might have modified parts of the file, and it is important for usermode to know.

No, I think error is more important than cerror.

In other words, it should be

if (error == 0)
     error = cerror;