Re: [PATCH 07/14] fuse: Update i_mtime on buffered writes

[Miklos Szeredi <miklos@szeredi.hu>]

On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
<MPatlasov@parallels.com> wrote:
> If writeback cache is on, buffered write doesn't result in immediate mtime
> update in userspace because the userspace will see modified data later, when
> writeback happens. Consequently, mtime provided by userspace may be older than
> actual time of buffered write.
>
> The problem can be solved by generating mtime locally (will come in next
> patches) and flushing it to userspace periodically. Here we introduce a flag to
> keep the state of fuse_inode: the flag is ON if and only if locally generated
> mtime (stored in inode->i_mtime) was not pushed to the userspace yet.
>
> The patch also implements all bits related to flushing and clearing the flag.
>
> Signed-off-by: Maxim Patlasov <MPatlasov@parallels.com>
> ---
>  fs/fuse/dir.c    |   42 +++++++++++++++++++++++++----
>  fs/fuse/file.c   |   31 ++++++++++++++++++---
>  fs/fuse/fuse_i.h |   13 ++++++++-
>  fs/fuse/inode.c  |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 154 insertions(+), 11 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index ff8b603..969c60d 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -177,6 +177,13 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
>                 if (flags & LOOKUP_RCU)
>                         return -ECHILD;
>
> +               if (test_bit(FUSE_I_MTIME_UPDATED,
> +                            &get_fuse_inode(inode)->state)) {
> +                       err = fuse_flush_mtime(inode, 0);

->d_revalidate may be called with or without i_mutex, there's
absolutely no way to know.  So this won't work.

I know it was me who suggested this approach, but I have second
thoughts...  I really don't like the way this mixes userspace and
kernel updates to mtime.  I think it should be either one or the
other.

I don't think you need to much changes to this patch.  Just clear
S_NOCMTIME, implement i_op->update_time(), which sets the
FUSE_I_MTIME_UPDATED flag and flush mtime just like you do now.
Except now it doesn't need to take i_mutex since all mtime updates are
now done by the kernel.

Does that make sense?

Thanks,
Miklos