I do not understand this. The new code calls vn_getsize_locked() both for size == 0 and size == VNODE_NO_SIZE, while the stated goal is to avoid vn_getsize_locked() call for size == 0.

You can reuse the vn_isdisk() check from there. E.g. this place might become

if (vn_isdisk(vp)) {
  if (size == 0 || isize == VNODE_NO_SIZE)
     size = IDX_TO_OFF(INT_MAX);
} else if (!vn_canvmio(vp)) {
    return (0);
}
and then vn_isdisk check below can be eliminated. vn_isdisk takes the devmtx.

BTW, can size == 0 for disks happen?  As I understand, this is to handle zero mediasize in g_vfs_open().  Is this possible?

You can reuse the vn_isdisk() check from there. E.g. this place might become

if (vn_isdisk(vp)) {
  if (size == 0 || isize == VNODE_NO_SIZE)
     size = IDX_TO_OFF(INT_MAX);
} else if (!vn_canvmio(vp)) {
    return (0);
}
and then vn_isdisk check below can be eliminated. vn_isdisk takes the devmtx.

Maybe it would be possible to set some flag on the vnode and avoid devmtx altogether?

BTW, can size == 0 for disks happen? As I understand, this is to handle zero mediasize in g_vfs_open(). Is this possible?

From what I remember, you can have mediasize equals zero when there is no media, eg. you have a cd-rom without CD inside.

BTW, can size == 0 for disks happen? As I understand, this is to handle zero mediasize in g_vfs_open(). Is this possible?

This is what happens when we try to open an empty cd0:

g_vfs_open:295: vp=0xfffff80006f52700 mediasize=0
vnode_create_vobject:160: vp=0xfffff80006f52700 size=0
g_vfs_done():cd0[READ(offset=65536, length=8192)]error = 6 supressing further ENXIO

I think we could just move the check for mediasize == 0 to g_vfs_open and return ENXIO there.

Why do we call g_access() before this check? Is read access required in order to keep pp->mediasize stable?

IMO, the name vnode_create_disk_vobject() keeps the namespace more consistent.

KASSERT(isize == VNODE_NO_SIZE || isize >= 0)?

Better use VNASSERT() there and in the next line.

That said, this assertion is too strong. Due to the devfs vnode lifecycle, the vnode lock is not enough to guarantee that vn_isdisk() returns true for disk.

Why do we call g_access() before this check? Is read access required in order to keep pp->mediasize stable?

The mediasize property can be set on first access. See g_disk_access().

That said, this assertion is too strong. Due to the devfs vnode lifecycle, the vnode lock is not enough to guarantee that vn_isdisk() returns true for disk.

Not sure if I understand this one. devfs doesn't create vm objects for vnodes. File systems goes to GEOM directly through g_vfs_* KPI.

devfs (destroy_dev(9)) might cause the assert to become false at any time.

I misplaced the comment though: the issue is that vn_isdisk() for disk might become false, not this one.

So basically when a process is trying to mount a file system while devfs entry is being destroyed? Makes sense. Thanks.