From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Maxim V. Patlasov" Subject: Re: [PATCH 6/10] fuse: Trust kernel i_size only Date: Mon, 1 Oct 2012 21:30:44 +0400 Message-ID: <5069D344.9090501@parallels.com> References: <4FF3156E.8030109@parallels.com> <4FF3160B.6090501@parallels.com> <8762a3pp3m.fsf@tucsk.pomaz.szeredi.hu> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Kirill Korotaev , Pavel Emelianov , "fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org" , James Bottomley , Alexander Viro , linux-fsdevel To: Miklos Szeredi Return-path: In-Reply-To: <8762a3pp3m.fsf-d8RdFUjzFsbxNFs70CDYszOMxtEWgIxa@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: fuse-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org Hi Miklos, 07/04/2012 06:39 PM, Miklos Szeredi ?????: > Pavel Emelyanov 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