On Tue, Jun 28, 2005 at 05:43:05PM +0200, Manuel Bouyer wrote:
> On Tue, Jun 28, 2005 at 06:59:53AM -0700, Chuck Silvers wrote:
> > On Tue, Jun 28, 2005 at 11:55:39AM +0200, Manuel Bouyer wrote:
> > > Hi,
> > > so here is another patch, based on comments received:
> > > - it also updates the mtime on VOP_PUTPAGE()
> > 
> > as I mentioned before, I don't think this makes any sense.
> > timestamp updates should be associated with changes to a file's
> > logical contents, not with internal cache-flushing activity.
> 
> Yes, but this requires an interface change in the vnode op, which may not
> be doable on the existing branches. In addition, this code doens't exist
> yet.

my point is that it does not seem correct to update timestamps as part of
cache-flushing.  whether or not the incorrect change is doable on a branch
is irrelevant.

it seems better to have VOP_GETPAGES() update atime on VM_PROT_READ accesses
and update mtime on VM_PROT_WRITE accesses.  the only caller that doesn't
fit with this behaviour that has been mentioned so far is ufs_balloc_range().


> > > - after fixing ufs_balloc_range() to not use VM_PROT_READ, test for
> > >   VM_PROT_READ in VOP_GETPAGE() to see if we should update the atime or not
> > 
> > is sorta makes more sense to use VM_PROT_WRITE here instead of VM_PROT_READ,
> 
> In a previous mail YAMAMOTO Takashi said it would be more straitforward to
> test VM_PROT_READ here instead of VM_PROT_WRITE (once ufs_balloc_range() uses
> VM_PROT_NONE).

I meant that it makes a bit more sense for ufs_balloc_range() to call
VOP_GETPAGES() with VM_PROT_WRITE instead of VM_PROT_READ.

-Chuck