Re: [PATCH 6/10] fuse: Trust kernel i_size only

["Maxim V. Patlasov" <mpatlasov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>]

Hi Miklos,

07/04/2012 06:39 PM, Miklos Szeredi ?????:
> Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> writes:
>
>> Make fuse think that when writeback is on the inode's i_size is alway
>> up-to-date and not update it with the value received from the
>> userspace. This is done because the page cache code may update i_size
>> without letting the FS know.
> Similar rule applies to i_mtime.  Except it's even more tricky, since
> you have to flush the mtime together with the data (the flush should not
> update the modification time).  And we have other operations which also
> change the mtime, and those also need to be updated.
>
> We should probably look at what NFS is doing.

In case of NFS, the flush does updates the modification time on server. 
And on client, getattr triggers flush:

> int nfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct 
> kstat *stat)
> {
>     ...
>
>     /* Flush out writes to the server in order to update c/mtime. */
>     if (S_ISREG(inode->i_mode)) {
>         nfs_inode_dio_wait(inode);
>         err = filemap_write_and_wait(inode->i_mapping);
>         if (err)
>             goto out;
>     }

In another email of this thread you suggested some approach where 
in-kernel fuse flushes i_mtime to userspace:

> So basically what we need is a per-inode flag that says that i_mtime has
> been updated (it is more recent then what userspace has) and we must
> update i_mtime*only*  in write and not other operations which still do
> the mtime update in the userspace filesystem.  Any operation that
> modifies i_mtime (and hence invalidate the attributes) must clear the
> flag.  Any other operation which updates or invalidates the attributes
> must first flush the i_mtime to userspace if the flag is set.
>
> In addition the userspace fileystem need to implement the policy similar
> to NFS, in which it only updates mtime if it is greater than the current
> one.  This means that we must differentiate between an mtime update due
> to a buffered write from an mtime update due to an utime (and friends)
> system call.

My question is why do we need all these complications if we could follow 
NFS way: trigger flush and wait for its (and fuse write-back) completion 
before sending FUSE_GETATTR to userspace?

Another concern is about the idea of sending i_mtime to userspace per 
se. You wrote:

> If we are doing buffered writes, then the kernel must update i_mtime on
> write(2) and must flush that to the userspace filesystem at some point
> (with a SETTATTR operation).

Fuse userspace may have its own non-trivial concept of 'modification 
time'. It's not obliged to advance its mtime on every write. The only 
requirement is to be consistent: if we expose new data handling READs, 
mtime must be advanced properly as well. But, for example, the 
granularity of changes is up to implementation. From this view, in-kenel 
fuse pushing i_mtime with a SETATTR operation would look like a cheating 
userspace. What do you think?

Thanks,
Maxim
------------------------------------------------------------------------------
Got visibility?
Most devs has no idea what their production app looks like.
Find out how fast your code is with AppDynamics Lite.
http://ad.doubleclick.net/clk;262219671;13503038;y?
http://info.appdynamics.com/FreeJavaPerformanceDownload.html