On 2/10/20 11:54 AM, Mateusz Guzik wrote: > On 2/3/20, John Baldwin <j...@freebsd.org> wrote: >> On 1/31/20 10:46 PM, Mateusz Guzik wrote: >>> Author: mjg >>> Date: Sat Feb 1 06:46:55 2020 >>> New Revision: 357361 >>> URL: https://svnweb.freebsd.org/changeset/base/357361 >>> >>> Log: >>> vfs: replace VOP_MARKATIME with VOP_MMAPPED >>> >>> The routine is only provided by ufs and is only used on mmap and exec. >>> >>> Reviewed by: kib >>> Differential Revision: https://reviews.freebsd.org/D23422 >>> >>> Modified: >>> head/sys/kern/kern_exec.c >>> head/sys/kern/vfs_subr.c >>> head/sys/kern/vnode_if.src >>> head/sys/sys/vnode.h >>> head/sys/ufs/ufs/ufs_vnops.c >>> head/sys/vm/vm_mmap.c >>> >>> Modified: head/sys/ufs/ufs/ufs_vnops.c >>> ============================================================================== >>> --- head/sys/ufs/ufs/ufs_vnops.c Sat Feb 1 06:41:44 2020 >>> (r357360) >>> +++ head/sys/ufs/ufs/ufs_vnops.c Sat Feb 1 06:46:55 2020 >>> (r357361) >>> @@ -108,7 +108,7 @@ static vop_getattr_t ufs_getattr; >>> static vop_ioctl_t ufs_ioctl; >>> static vop_link_t ufs_link; >>> static int ufs_makeinode(int mode, struct vnode *, struct vnode **, >>> struct componentname *, const char *); >>> -static vop_markatime_t ufs_markatime; >>> +static vop_mmapped_t ufs_mmapped; >>> static vop_mkdir_t ufs_mkdir; >>> static vop_mknod_t ufs_mknod; >>> static vop_open_t ufs_open; >>> @@ -676,19 +676,22 @@ out: >>> } >>> #endif /* UFS_ACL */ >>> >>> -/* >>> - * Mark this file's access time for update for vfs_mark_atime(). This >>> - * is called from execve() and mmap(). >>> - */ >> >> Why remove this comment rather than update it? It is largely still >> true and explains the purpose of the VOP (update the atime) which is >> now no longer obvious from the name. >> > > I don't think a fs-specific implementation of a VOP is the right place to > state where it is called from. I would argue the name could be better as > the execve bit is definitely not obvious, but interested parties can > always grep.
This (always grep) is why comments matter. If we wanted people to just use grep and always UTSL, we wouldn't bother having any comments at all. One of the purposes of comments is to allow a human reader to understand the code in context without needing several different windows open to piece together what is happening. In particular, the comment gives a better hint as to _why_ we are updating the atime. The old name did a better job of this than the new one which is more bland, but presumably you have future changes to add that are beyond just changing the atime and that is why you renamed it? -- John Baldwin _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
