linux-kernel - Re: [PATCH 07/14] fuse: Update i

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJfpegtszx6cDJ9fUPTB_z-1vG7iM8J1jdKuEdqN+wPFkOf6NQ@mail.gmail.com>
Date:	Tue, 29 Jan 2013 23:19:40 +0100
From:	Miklos Szeredi <miklos@...redi.hu>
To:	"Maxim V. Patlasov" <MPatlasov@...allels.com>
Cc:	dev@...allels.com, xemul@...allels.com,
	fuse-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
	jbottomley@...allels.com, viro@...iv.linux.org.uk,
	linux-fsdevel@...r.kernel.org, devel@...nvz.org
Subject: Re: [PATCH 07/14] fuse: Update i_mtime on buffered writes

On Fri, Jan 25, 2013 at 7:24 PM, Maxim V. Patlasov
<MPatlasov@...allels.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@...allels.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/