Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Feb 2020 20:54:34 +0100
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r357361 - in head/sys: kern sys ufs/ufs vm
Message-ID:  <CAGudoHHYBzn6mQzdUzuy7zQfh7D60JQqQ=TcFaRFn7i-cR76Sw@mail.gmail.com>
In-Reply-To: <94dd2422-307b-9c06-ad84-d13d2e8a9fa4@FreeBSD.org>
References:  <202002010646.0116ktUk057327@repo.freebsd.org> <94dd2422-307b-9c06-ad84-d13d2e8a9fa4@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 2/3/20, John Baldwin <jhb@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.

Finally, the function literally just updates atime so I don't think
repeating this in the comment buys anything.

-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHYBzn6mQzdUzuy7zQfh7D60JQqQ=TcFaRFn7i-cR76Sw>