Re: [Cluster-devel] [PATCH v7 05/13] fat: make fat_update_time get its own timestamp

On Thu, 2023-08-10 at 03:31 +0900, OGAWA Hirofumi wrote:
> Jeff Layton <[email protected]> writes:
> 
> > On Thu, 2023-08-10 at 02:44 +0900, OGAWA Hirofumi wrote:
> > > Jeff Layton <[email protected]> writes:
> > > 
> > That would be wrong. The problem is that we're changing how update_time
> > works:
> > 
> > Previously, update_time was given a timestamp and a set of S_* flags to
> > indicate which fields should be updated. Now, update_time is not given a
> > timestamp. It needs to fetch it itself, but that subtly changes the
> > meaning of the flags field.
> > 
> > It now means "these fields needed to be updated when I last checked".
> > The timestamp and i_version may now be different from when the flags
> > field was set. This means that if any of S_CTIME/S_MTIME/S_VERSION were
> > set that we need to attempt to update all 3 of them. They may now be
> > different from the timestamp or version that we ultimately end up with.
> > 
> > The above may look to you like it would always cause I_DIRTY_SYNC to be
> > set on any ctime or mtime update, but inode_maybe_inc_iversion only
> > returns true if it actually updated i_version, and it only does that if
> > someone issued a ->getattr against the file since the last time it was
> > updated.
> > 
> > So, this shouldn't generate any more DIRTY_SYNC updates than it did
> > before.
> 
> Again, if you claim so, why generic_update_time() doesn't work same? Why
> only FAT does?
> 
> Or I'm misreading generic_update_time() patch?
> 

When you say it "doesn't work the same", what do you mean, specifically?
I had to make some allowances for the fact that FAT is substantially
different in its timestamp handling, and I tried to preserve existing
behavior as best I could.

-- 
Jeff Layton <[email protected]>