This is half-done. Either you should call vnops.fo_ioctl() instead of VOP_IOCTL() (see e.g. devfs_close_f() how it is done), or you do not need VOP_IOCTL() call there at all, instead directly dispatching the calls to appropriate routines from this file.

My preference is to redirect to vnops.fo_ioctl, it would collapse error !=0 and error == 0 cases.

Also, I do not see why do you need devfs_fp_check() in the file-level method.

Why vnops.fo_ioctl over VOP_IOCTL()? The only cases it handles today are VDIR and VREG, while these are character devices.

Also, I do not see why do you need devfs_fp_check() in the file-level method.

At the vnode layer, we may not have a file to check. So the check is done at the appropriate layer.

Calling VOP_XXX() in the filesystem code for its own vnode is silly. This and the fact that vnops.fo_ioctl() does some additional handling, which is relevant now for VDIR, and might be relevant for other cases in future.

My point is that you do not need devfs_fp_check(), the file method only should handle cdevpriv.

It would be cleaner to check for errors earlier, returning to the previous nesting level type.

error = devfs_vn_check();
if (error != 0)
  return (error);
if (*devvp != fp->f_data)
   return (ENXIO);
curthread->td_fpop = fp;
return (0);

And no need to check for *dswp != NULL, since devfs_vn_check() does not return success and NULL *dswp.

In fact, after this simplification,ee a reason for devfs_vn_check. Just use devvn_refthread() in devfs_ioctl() directly and things become much simpler.

Ok.

Did you ever boot the system with these changes ? All VCHR io should abort with ENXIO.

Just keep devfs_fp_check() intact.

I do not see why this rearrangement of devfs_fp_check is needed or useful. Drop it.

Otherwise the patch looks fine.