2014-04-15 11:27:54

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 0/7] fuse: fix handling mtime and cmtime in writeback_cache mode

Hi,

The patch-set fixes a few issues introduced by writeback_cache feature:

- fuse_iget() used inode->i_mode before its initialization
- in course of truncate(2), inode->i_mtime was not updated at all
- the same for open(O_TRUNC) in case of fc->atomic_o_trunc is set
- we cannot trust ctime from server anymore because we doesn't
flush all data changes to the server immediately
- to maintain ctime locally, we have to update it in proper places
(fuse_update_time, fallocate, setxattr, etc.)
- locally updated ctime must be flushed to the server eventually.

Maxim
---

Maxim Patlasov (7):
fuse: do not use uninitialized i_mode
fuse: update mtime on truncate(2)
fuse: update mtime on open(O_TRUNC) in atomic_o_trunc mode
fuse: introduce FUSE_I_CTIME_DIRTY flag
fuse: trust kernel i_ctime only
fuse: clear FUSE_I_CTIME_DIRTY flag on setattr
fuse: flush ctime in FUSE_FORGET requests


fs/fuse/dev.c | 30 ++++++++++++---
fs/fuse/dir.c | 88 +++++++++++++++++++++++++++++++++++++++------
fs/fuse/file.c | 37 +++++++++++++------
fs/fuse/fuse_i.h | 6 ++-
fs/fuse/inode.c | 14 ++++++-
include/uapi/linux/fuse.h | 29 +++++++++++++--
6 files changed, 165 insertions(+), 39 deletions(-)

--
Signature


2014-04-15 11:28:14

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 1/7] fuse: do not use uninitialized i_mode

When inode is in I_NEW state, inode->i_mode is not initialized yet. Do not
use it before fuse_init_inode() is called.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8d61169..299e553 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -303,7 +303,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,

if ((inode->i_state & I_NEW)) {
inode->i_flags |= S_NOATIME;
- if (!fc->writeback_cache || !S_ISREG(inode->i_mode))
+ if (!fc->writeback_cache || !S_ISREG(attr->mode))
inode->i_flags |= S_NOCMTIME;
inode->i_generation = generation;
inode->i_data.backing_dev_info = &fc->bdi;

2014-04-15 11:28:55

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 2/7] fuse: update mtime on truncate(2)

Handling truncate(2), VFS doesn't set ATTR_MTIME bit in iattr structure; only
ATTR_SIZE bit is set. In-kernel fuse must handle the case by setting mtime
fields of struct fuse_setattr_in to "now" and set FATTR_MTIME bit even
though ATTR_MTIME was not set.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/dir.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 5b4e035..c7cb41c 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1503,10 +1503,11 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
return true;
}

-static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
- bool trust_local_mtime)
+static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
+ struct fuse_setattr_in *arg, bool trust_local_mtime)
{
unsigned ivalid = iattr->ia_valid;
+ struct timespec now = current_fs_time(inode->i_sb);

if (ivalid & ATTR_MODE)
arg->valid |= FATTR_MODE, arg->mode = iattr->ia_mode;
@@ -1529,6 +1530,10 @@ static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
arg->mtimensec = iattr->ia_mtime.tv_nsec;
if (!(ivalid & ATTR_MTIME_SET) && !trust_local_mtime)
arg->valid |= FATTR_MTIME_NOW;
+ } else if ((ivalid & ATTR_SIZE) && trust_local_mtime) {
+ arg->valid |= FATTR_MTIME;
+ arg->mtime = now.tv_sec;
+ arg->mtimensec = now.tv_nsec;
}
}

@@ -1682,7 +1687,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,

memset(&inarg, 0, sizeof(inarg));
memset(&outarg, 0, sizeof(outarg));
- iattr_to_fattr(attr, &inarg, trust_local_mtime);
+ iattr_to_fattr(inode, attr, &inarg, trust_local_mtime);
if (file) {
struct fuse_file *ff = file->private_data;
inarg.valid |= FATTR_FH;
@@ -1711,8 +1716,9 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,

spin_lock(&fc->lock);
/* the kernel maintains i_mtime locally */
- if (trust_local_mtime && (attr->ia_valid & ATTR_MTIME)) {
- inode->i_mtime = attr->ia_mtime;
+ if (trust_local_mtime && (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE))) {
+ inode->i_mtime.tv_sec = inarg.mtime;
+ inode->i_mtime.tv_nsec = inarg.mtimensec;
clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
}

2014-04-15 11:29:35

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 3/7] fuse: update mtime on open(O_TRUNC) in atomic_o_trunc mode

In case of fc->atomic_o_trunc is set, fuse does nothing in fuse_do_setattr()
while handling open(O_TRUNC). Hence, i_mtime must be updated explicitly in
fuse_finish_open(). The patch also adds extra locking encompassing
open(O_TRUNC) operation to avoid races between updating i_mtime and setting
FUSE_I_MTIME_DIRTY flag.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/file.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 13f8bde..c2c6df7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -223,6 +223,10 @@ void fuse_finish_open(struct inode *inode, struct file *file)
i_size_write(inode, 0);
spin_unlock(&fc->lock);
fuse_invalidate_attr(inode);
+ if (fc->writeback_cache) {
+ inode->i_mtime = current_fs_time(inode->i_sb);
+ set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ }
}
if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
fuse_link_write_file(file);
@@ -232,18 +236,26 @@ int fuse_open_common(struct inode *inode, struct file *file, bool isdir)
{
struct fuse_conn *fc = get_fuse_conn(inode);
int err;
+ bool lock_inode = (file->f_flags & O_TRUNC) &&
+ fc->atomic_o_trunc &&
+ fc->writeback_cache;

err = generic_file_open(inode, file);
if (err)
return err;

+ if (lock_inode)
+ mutex_lock(&inode->i_mutex);
+
err = fuse_do_open(fc, get_node_id(inode), file, isdir);
- if (err)
- return err;

- fuse_finish_open(inode, file);
+ if (!err)
+ fuse_finish_open(inode, file);

- return 0;
+ if (lock_inode)
+ mutex_unlock(&inode->i_mutex);
+
+ return err;
}

static void fuse_prepare_release(struct fuse_file *ff, int flags, int opcode)

2014-04-15 11:30:47

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 4/7] fuse: introduce FUSE_I_CTIME_DIRTY flag

The flag plays the role similar to FUSE_I_MTIME_DIRTY: local ctime
(inode->i_ctime) is newer than on the server; so, local ctime must be
flushed to the server afterwards.

The patch only introduces the flag, extends API (fuse_setattr_in) properly,
and implements the flush procedure (fuse_flush_cmtime()). Further patches of
the patch-set will implement the logic of setting and clearing the flag.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/dir.c | 22 ++++++++++++++++++----
fs/fuse/file.c | 11 ++++-------
fs/fuse/fuse_i.h | 4 +++-
include/uapi/linux/fuse.h | 11 ++++++++---
4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c7cb41c..0596726 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1600,9 +1600,9 @@ static void fuse_setattr_fill(struct fuse_conn *fc, struct fuse_req *req,
}

/*
- * Flush inode->i_mtime to the server
+ * Flush inode->i_mtime and inode->i_ctime to the server
*/
-int fuse_flush_mtime(struct file *file, bool nofail)
+int fuse_flush_cmtime(struct file *file, bool nofail)
{
struct inode *inode = file->f_mapping->host;
struct fuse_inode *fi = get_fuse_inode(inode);
@@ -1610,8 +1610,16 @@ int fuse_flush_mtime(struct file *file, bool nofail)
struct fuse_req *req = NULL;
struct fuse_setattr_in inarg;
struct fuse_attr_out outarg;
+ unsigned cmtime_flags = 0;
int err;

+ if (test_bit(FUSE_I_MTIME_DIRTY, &fi->state))
+ cmtime_flags |= FATTR_MTIME;
+ if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state) && fc->minor >= 24)
+ cmtime_flags |= FATTR_CTIME;
+ if (!cmtime_flags)
+ return 0;
+
if (nofail) {
req = fuse_get_req_nofail_nopages(fc, file);
} else {
@@ -1623,17 +1631,23 @@ int fuse_flush_mtime(struct file *file, bool nofail)
memset(&inarg, 0, sizeof(inarg));
memset(&outarg, 0, sizeof(outarg));

- inarg.valid |= FATTR_MTIME;
+ inarg.valid = cmtime_flags;
inarg.mtime = inode->i_mtime.tv_sec;
inarg.mtimensec = inode->i_mtime.tv_nsec;
+ if (fc->minor >= 24) {
+ inarg.ctime = inode->i_ctime.tv_sec;
+ inarg.ctimensec = inode->i_ctime.tv_nsec;
+ }

fuse_setattr_fill(fc, req, inode, &inarg, &outarg);
fuse_request_send(fc, req);
err = req->out.h.error;
fuse_put_request(fc, req);

- if (!err)
+ if (!err) {
clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ clear_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+ }

return err;
}
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index c2c6df7..9ed8590b 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -328,8 +328,7 @@ static int fuse_release(struct inode *inode, struct file *file)
if (fc->writeback_cache)
filemap_write_and_wait(file->f_mapping);

- if (test_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state))
- fuse_flush_mtime(file, true);
+ fuse_flush_cmtime(file, true);

fuse_release_common(file, FUSE_RELEASE);

@@ -512,11 +511,9 @@ int fuse_fsync_common(struct file *file, loff_t start, loff_t end,

fuse_sync_writes(inode);

- if (test_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state)) {
- int err = fuse_flush_mtime(file, false);
- if (err)
- goto out;
- }
+ err = fuse_flush_cmtime(file, false);
+ if (err)
+ goto out;

req = fuse_get_req_nopages(fc);
if (IS_ERR(req)) {
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index a257ed8e..781a01d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -121,6 +121,8 @@ enum {
FUSE_I_SIZE_UNSTABLE,
/** i_mtime has been updated locally; a flush to userspace needed */
FUSE_I_MTIME_DIRTY,
+ /** i_ctime has been updated locally; a flush to userspace needed */
+ FUSE_I_CTIME_DIRTY,
};

struct fuse_conn;
@@ -891,7 +893,7 @@ int fuse_dev_release(struct inode *inode, struct file *file);

bool fuse_write_update_size(struct inode *inode, loff_t pos);

-int fuse_flush_mtime(struct file *file, bool nofail);
+int fuse_flush_cmtime(struct file *file, bool nofail);

int fuse_do_setattr(struct inode *inode, struct iattr *attr,
struct file *file);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index cf4750e..8af06cc 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -96,6 +96,10 @@
*
* 7.23
* - add FUSE_WRITEBACK_CACHE
+ *
+ * 7.24
+ * - ctime support for FUSE_WRITEBACK_CACHE:
+ * changes in fuse_setattr_in and fuse_forget_in
*/

#ifndef _LINUX_FUSE_H
@@ -131,7 +135,7 @@
#define FUSE_KERNEL_VERSION 7

/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 23
+#define FUSE_KERNEL_MINOR_VERSION 24

/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -191,6 +195,7 @@ struct fuse_file_lock {
#define FATTR_ATIME_NOW (1 << 7)
#define FATTR_MTIME_NOW (1 << 8)
#define FATTR_LOCKOWNER (1 << 9)
+#define FATTR_CTIME (1 << 10)

/**
* Flags returned by the OPEN request
@@ -438,10 +443,10 @@ struct fuse_setattr_in {
uint64_t lock_owner;
uint64_t atime;
uint64_t mtime;
- uint64_t unused2;
+ uint64_t ctime;
uint32_t atimensec;
uint32_t mtimensec;
- uint32_t unused3;
+ uint32_t ctimensec;
uint32_t mode;
uint32_t unused4;
uint32_t uid;

2014-04-15 11:31:44

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 5/7] fuse: trust kernel i_ctime only

Let the kernel maintain i_ctime locally:
- clear S_NOCMTIME (implemented earlier)
- update i_ctime in i_op->update_time()
- flush ctime on fsync and last close (previous patch)
- update i_ctime explicitly on truncate, fallocate, open(O_TRUNC),
setxattr, link, rename, unlink.

Fuse inode flag FUSE_I_CTIME_DIRTY serves as indication that local i_ctime
should be flushed to the server eventually. The patch sets the flag and updates
i_ctime in course of operations listed above.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/dir.c | 24 +++++++++++++++++++++++-
fs/fuse/file.c | 8 ++++++--
fs/fuse/inode.c | 6 ++++--
3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 0596726..2187960 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -679,6 +679,15 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
return create_new_entry(fc, req, dir, entry, S_IFLNK);
}

+static inline void fuse_update_ctime(struct fuse_conn *fc, struct inode *inode)
+{
+ if (fc->writeback_cache && S_ISREG(inode->i_mode)) {
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ inode->i_ctime = current_fs_time(inode->i_sb);
+ set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+ }
+}
+
static int fuse_unlink(struct inode *dir, struct dentry *entry)
{
int err;
@@ -713,6 +722,7 @@ static int fuse_unlink(struct inode *dir, struct dentry *entry)
fuse_invalidate_attr(inode);
fuse_invalidate_attr(dir);
fuse_invalidate_entry_cache(entry);
+ fuse_update_ctime(fc, inode);
} else if (err == -EINTR)
fuse_invalidate_entry(entry);
return err;
@@ -771,6 +781,7 @@ static int fuse_rename(struct inode *olddir, struct dentry *oldent,
if (!err) {
/* ctime changes */
fuse_invalidate_attr(oldent->d_inode);
+ fuse_update_ctime(fc, oldent->d_inode);

fuse_invalidate_attr(olddir);
if (olddir != newdir)
@@ -780,6 +791,7 @@ static int fuse_rename(struct inode *olddir, struct dentry *oldent,
if (newent->d_inode) {
fuse_invalidate_attr(newent->d_inode);
fuse_invalidate_entry_cache(newent);
+ fuse_update_ctime(fc, newent->d_inode);
}
} else if (err == -EINTR) {
/* If request was interrupted, DEITY only knows if the
@@ -829,6 +841,7 @@ static int fuse_link(struct dentry *entry, struct inode *newdir,
inc_nlink(inode);
spin_unlock(&fc->lock);
fuse_invalidate_attr(inode);
+ fuse_update_ctime(fc, inode);
} else if (err == -EINTR) {
fuse_invalidate_attr(inode);
}
@@ -846,6 +859,8 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
attr->size = i_size_read(inode);
attr->mtime = inode->i_mtime.tv_sec;
attr->mtimensec = inode->i_mtime.tv_nsec;
+ attr->ctime = inode->i_ctime.tv_sec;
+ attr->ctimensec = inode->i_ctime.tv_nsec;
}

stat->dev = inode->i_sb->s_dev;
@@ -1830,8 +1845,10 @@ static int fuse_setxattr(struct dentry *entry, const char *name,
fc->no_setxattr = 1;
err = -EOPNOTSUPP;
}
- if (!err)
+ if (!err) {
fuse_invalidate_attr(inode);
+ fuse_update_ctime(fc, inode);
+ }
return err;
}

@@ -1974,6 +1991,11 @@ static int fuse_update_time(struct inode *inode, struct timespec *now,
set_bit(FUSE_I_MTIME_DIRTY, &get_fuse_inode(inode)->state);
BUG_ON(!S_ISREG(inode->i_mode));
}
+ if (flags & S_CTIME) {
+ inode->i_ctime = *now;
+ set_bit(FUSE_I_CTIME_DIRTY, &get_fuse_inode(inode)->state);
+ BUG_ON(!S_ISREG(inode->i_mode));
+ }
return 0;
}

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 9ed8590b..f644aeb 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -224,8 +224,10 @@ void fuse_finish_open(struct inode *inode, struct file *file)
spin_unlock(&fc->lock);
fuse_invalidate_attr(inode);
if (fc->writeback_cache) {
- inode->i_mtime = current_fs_time(inode->i_sb);
+ inode->i_ctime = inode->i_mtime =
+ current_fs_time(inode->i_sb);
set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
}
}
if ((file->f_mode & FMODE_WRITE) && fc->writeback_cache)
@@ -3029,8 +3031,10 @@ static long fuse_file_fallocate(struct file *file, int mode, loff_t offset,
if (changed && fc->writeback_cache) {
struct fuse_inode *fi = get_fuse_inode(inode);

- inode->i_mtime = current_fs_time(inode->i_sb);
+ inode->i_ctime = inode->i_mtime =
+ current_fs_time(inode->i_sb);
set_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ set_bit(FUSE_I_CTIME_DIRTY, &fi->state);
}
}

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 299e553..8b9a1d1 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -175,9 +175,9 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
if (!fc->writeback_cache || !S_ISREG(inode->i_mode)) {
inode->i_mtime.tv_sec = attr->mtime;
inode->i_mtime.tv_nsec = attr->mtimensec;
+ inode->i_ctime.tv_sec = attr->ctime;
+ inode->i_ctime.tv_nsec = attr->ctimensec;
}
- inode->i_ctime.tv_sec = attr->ctime;
- inode->i_ctime.tv_nsec = attr->ctimensec;

if (attr->blksize != 0)
inode->i_blkbits = ilog2(attr->blksize);
@@ -256,6 +256,8 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
inode->i_size = attr->size;
inode->i_mtime.tv_sec = attr->mtime;
inode->i_mtime.tv_nsec = attr->mtimensec;
+ inode->i_ctime.tv_sec = attr->ctime;
+ inode->i_ctime.tv_nsec = attr->ctimensec;
if (S_ISREG(inode->i_mode)) {
fuse_init_common(inode);
fuse_init_file_inode(inode);

2014-04-15 11:32:21

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 6/7] fuse: clear FUSE_I_CTIME_DIRTY flag on setattr

The patch addresses two use-cases when the flag may be safely cleared:

1. fuse_do_setattr() is called with ATTR_CTIME flag set in attr->ia_valid.
In this case attr->ia_ctime bears actual value. In-kernel fuse must send it
to the userspace server and then assign the value to inode->i_ctime.

2. fuse_do_setattr() is called with ATTR_SIZE flag set in attr->ia_valid,
whereas ATTR_CTIME is not set (truncate(2)).
In this case in-kernel fuse must sent "now" to the userspace server and then
assign the value to inode->i_ctime.

In both cases fuse can clear FUSE_I_CTIME_DIRTY flag because actual ctime
value was flushed to the server.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/dir.c | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 2187960..3495aaf 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1518,8 +1518,9 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
return true;
}

-static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
- struct fuse_setattr_in *arg, bool trust_local_mtime)
+static void iattr_to_fattr(struct fuse_conn *fc, struct inode *inode,
+ struct iattr *iattr, struct fuse_setattr_in *arg,
+ bool trust_local_mtime, struct timespec *ctime)
{
unsigned ivalid = iattr->ia_valid;
struct timespec now = current_fs_time(inode->i_sb);
@@ -1550,6 +1551,19 @@ static void iattr_to_fattr(struct inode *inode, struct iattr *iattr,
arg->mtime = now.tv_sec;
arg->mtimensec = now.tv_nsec;
}
+
+ if ((ivalid & ATTR_CTIME) && trust_local_mtime)
+ *ctime = iattr->ia_ctime;
+ else if ((ivalid & ATTR_SIZE) && trust_local_mtime)
+ *ctime = now;
+ else
+ ctime = NULL;
+
+ if (ctime && fc->minor >= 24) {
+ arg->valid |= FATTR_CTIME;
+ arg->ctime = ctime->tv_sec;
+ arg->ctimensec = ctime->tv_nsec;
+ }
}

/*
@@ -1688,6 +1702,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
loff_t oldsize;
int err;
bool trust_local_mtime = is_wb && S_ISREG(inode->i_mode);
+ struct timespec ctime;

if (!(fc->flags & FUSE_DEFAULT_PERMISSIONS))
attr->ia_valid |= ATTR_FORCE;
@@ -1716,7 +1731,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,

memset(&inarg, 0, sizeof(inarg));
memset(&outarg, 0, sizeof(outarg));
- iattr_to_fattr(inode, attr, &inarg, trust_local_mtime);
+ iattr_to_fattr(fc, inode, attr, &inarg, trust_local_mtime, &ctime);
if (file) {
struct fuse_file *ff = file->private_data;
inarg.valid |= FATTR_FH;
@@ -1744,11 +1759,18 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
}

spin_lock(&fc->lock);
- /* the kernel maintains i_mtime locally */
- if (trust_local_mtime && (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE))) {
- inode->i_mtime.tv_sec = inarg.mtime;
- inode->i_mtime.tv_nsec = inarg.mtimensec;
- clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ /* the kernel maintains i_mtime and i_ctime locally */
+ if (trust_local_mtime) {
+ if (attr->ia_valid & (ATTR_MTIME | ATTR_SIZE)) {
+ inode->i_mtime.tv_sec = inarg.mtime;
+ inode->i_mtime.tv_nsec = inarg.mtimensec;
+ clear_bit(FUSE_I_MTIME_DIRTY, &fi->state);
+ }
+
+ if (attr->ia_valid & (ATTR_CTIME | ATTR_SIZE)) {
+ inode->i_ctime = ctime;
+ clear_bit(FUSE_I_CTIME_DIRTY, &fi->state);
+ }
}

fuse_change_attributes_common(inode, &outarg.attr,

2014-04-15 11:33:08

by Maxim Patlasov

[permalink] [raw]
Subject: [PATCH 7/7] fuse: flush ctime in FUSE_FORGET requests

Some inode operations (e.g., rename) operate directly on inodes and dentries
without opened files involved. This means that even though fuse set
inode->i_ctime and FUSE_I_CTIME_DIRTY properly, a corresponding flush operation
will never happen (i.e. no fsync or close to call fuse_flush_cmtime()).

The patch solves the problem by passing local ctime to the userspace server
inside forget requests.

Signed-off-by: Maxim Patlasov <[email protected]>
---
fs/fuse/dev.c | 30 +++++++++++++++++++++++-------
fs/fuse/fuse_i.h | 2 +-
fs/fuse/inode.c | 6 ++++++
include/uapi/linux/fuse.h | 18 ++++++++++++++++++
4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index aac71ce..97c7749 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -578,17 +578,25 @@ void fuse_request_send_background_locked(struct fuse_conn *fc,
void fuse_force_forget(struct file *file, u64 nodeid)
{
struct inode *inode = file_inode(file);
+ struct fuse_inode *fi = get_fuse_inode(inode);
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req;
- struct fuse_forget_in inarg;
+ struct fuse_forget_in_ext inarg;
+ int inarg_size = fc->minor >= 24 ?
+ sizeof(inarg) : sizeof(struct fuse_forget_in);

memset(&inarg, 0, sizeof(inarg));
inarg.nlookup = 1;
+ if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state)) {
+ inarg.forget_flags = FUSE_FORGET_CTIME_SET;
+ inarg.ctime = inode->i_ctime.tv_sec;
+ inarg.ctimensec = inode->i_ctime.tv_nsec;
+ }
req = fuse_get_req_nofail_nopages(fc, file);
req->in.h.opcode = FUSE_FORGET;
req->in.h.nodeid = nodeid;
req->in.numargs = 1;
- req->in.args[0].size = sizeof(inarg);
+ req->in.args[0].size = inarg_size;
req->in.args[0].value = &inarg;
req->isreply = 0;
__fuse_request_send(fc, req);
@@ -1102,14 +1110,19 @@ __releases(fc->lock)
{
int err;
struct fuse_forget_link *forget = dequeue_forget(fc, 1, NULL);
- struct fuse_forget_in arg = {
+ struct fuse_forget_in_ext arg = {
.nlookup = forget->forget_one.nlookup,
+ .ctime = forget->forget_one.ctime,
+ .ctimensec = forget->forget_one.ctimensec,
+ .forget_flags = forget->forget_one.forget_flags,
};
+ int arg_size = fc->minor >= 24 ?
+ sizeof(arg) : sizeof(struct fuse_forget_in);
struct fuse_in_header ih = {
.opcode = FUSE_FORGET,
.nodeid = forget->forget_one.nodeid,
.unique = fuse_get_unique(fc),
- .len = sizeof(ih) + sizeof(arg),
+ .len = sizeof(ih) + arg_size,
};

spin_unlock(&fc->lock);
@@ -1142,18 +1155,21 @@ __releases(fc->lock)
.unique = fuse_get_unique(fc),
.len = sizeof(ih) + sizeof(arg),
};
+ int forget_one_size = fc->minor >= 24 ?
+ sizeof(struct fuse_forget_one_ext) :
+ sizeof(struct fuse_forget_one);

if (nbytes < ih.len) {
spin_unlock(&fc->lock);
return -EINVAL;
}

- max_forgets = (nbytes - ih.len) / sizeof(struct fuse_forget_one);
+ max_forgets = (nbytes - ih.len) / forget_one_size;
head = dequeue_forget(fc, max_forgets, &count);
spin_unlock(&fc->lock);

arg.count = count;
- ih.len += count * sizeof(struct fuse_forget_one);
+ ih.len += count * forget_one_size;
err = fuse_copy_one(cs, &ih, sizeof(ih));
if (!err)
err = fuse_copy_one(cs, &arg, sizeof(arg));
@@ -1163,7 +1179,7 @@ __releases(fc->lock)

if (!err) {
err = fuse_copy_one(cs, &forget->forget_one,
- sizeof(forget->forget_one));
+ forget_one_size);
}
head = forget->next;
kfree(forget);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 781a01d..d5172fa 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -59,7 +59,7 @@ extern unsigned max_user_congthresh;

/* One forget request */
struct fuse_forget_link {
- struct fuse_forget_one forget_one;
+ struct fuse_forget_one_ext forget_one;
struct fuse_forget_link *next;
};

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 8b9a1d1..932e04a 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -128,6 +128,12 @@ static void fuse_evict_inode(struct inode *inode)
if (inode->i_sb->s_flags & MS_ACTIVE) {
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_inode *fi = get_fuse_inode(inode);
+ if (test_bit(FUSE_I_CTIME_DIRTY, &fi->state)) {
+ struct fuse_forget_link *fl = fi->forget;
+ fl->forget_one.forget_flags = FUSE_FORGET_CTIME_SET;
+ fl->forget_one.ctime = inode->i_ctime.tv_sec;
+ fl->forget_one.ctimensec = inode->i_ctime.tv_nsec;
+ }
fuse_queue_forget(fc, fi->forget, fi->nodeid, fi->nlookup);
fi->forget = NULL;
}
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 8af06cc..5354a8c 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -388,6 +388,24 @@ struct fuse_forget_in {
uint64_t nlookup;
};

+struct fuse_forget_in_ext {
+ uint64_t nlookup;
+ uint64_t ctime;
+ uint32_t ctimensec;
+ uint32_t forget_flags;
+};
+
+/* forget_flags */
+#define FUSE_FORGET_CTIME_SET (1 << 0)
+
+struct fuse_forget_one_ext {
+ uint64_t nodeid;
+ uint64_t nlookup;
+ uint64_t ctime;
+ uint32_t ctimensec;
+ uint32_t forget_flags;
+};
+
struct fuse_forget_one {
uint64_t nodeid;
uint64_t nlookup;

2014-04-28 14:51:48

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 7/7] fuse: flush ctime in FUSE_FORGET requests

Maxim,

Thanks for the patches.

On Tue, Apr 15, 2014 at 1:33 PM, Maxim Patlasov <[email protected]> wrote:
> Some inode operations (e.g., rename) operate directly on inodes and dentries
> without opened files involved. This means that even though fuse set
> inode->i_ctime and FUSE_I_CTIME_DIRTY properly, a corresponding flush operation
> will never happen (i.e. no fsync or close to call fuse_flush_cmtime()).
>
> The patch solves the problem by passing local ctime to the userspace server
> inside forget requests.

Hmm, I really don't like this.

1) What has forget to do with ctime? It feels like being forced into
the interface
2) Forget may not be called for a long time after the modification and
it may not be called *at all*, which would result in the loss of the
ctime change after umount.

How about wiring up fuse_flush_cmtime() to be called from
s_op->write_inode() instead?

Updated patchset pushed to

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-linus

Survives some basic testing, but it would be great if you could also
take a look.

Thanks,
Miklos