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

["Maxim V. Patlasov" <mpatlasov@parallels.com>]

Hi Miklos,

Thanks a lot for reply. See please inline comments below...

11/16/2012 01:49 PM, Miklos Szeredi пишет:
> "Maxim V. Patlasov" <mpatlasov@parallels.com> writes:
>
>>      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?
>
> Yes, the NFS way seems like a good approach assuming that getattrs are
> not too frequent.  But I guess the fact that NFS does this is a pretty
> good assurance that will work fine.

My first intention was to follow NFS way because it's very simple and 
straightforward. But then I realized it would impact performance too 
badly. You correctly noticed that frequent getattrs will be a problem. 
But there will be a problem even without it: a single innocent 'ls' will 
wait for pretty long till all dirty memory is flushed.


>> 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?
> I think you are right in that mixing kernel mtime updates with userspace
> mtime updates doesn't work.  Either the kernel should be wholly
> responsible (which works only for "local" filesystems) or the userspace
> is fully responsible for mtime updates (which works in all cases but may
> be suboptimal).

Not having any feedback from you for long while I worked pretty hard to 
implement the approach you suggested early (update mtime locally on 
buffered writes, flush it to userspace when needed). Now I have an 
implementation that works in my tests. I'll send patches soon.

Thanks,
Maxim
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html