All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] virtiofs,fuse: support per-file DAX
@ 2021-07-15  9:30 ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, virtualization, bo.liu

This patchset adds support of per-file DAX for virtiofs, which is
inspired by Ira Weiny's work on ext4[1] and xfs[2].

Currently virtiofs (in guest kernel) accepts per-file DAX flag from FUSE
server (in host). 

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Any comment is welcome. :)


[1] commit 9cb20f94afcd ("fs/ext4: Make DAX mount option a tri-state")
[2] commit 02beb2686ff9 ("fs/xfs: Make DAX mount option a tri-state")

Jeffle Xu (3):
  fuse: add fuse_should_enable_dax() helper
  fuse: Make DAX mount option a tri-state
  fuse: add per-file DAX flag

 fs/fuse/dax.c             | 43 +++++++++++++++++++++++++++++++++++++--
 fs/fuse/file.c            |  4 ++--
 fs/fuse/fuse_i.h          | 16 +++++++++++----
 fs/fuse/inode.c           |  6 ++++--
 fs/fuse/virtio_fs.c       | 16 +++++++++++++--
 include/uapi/linux/fuse.h |  5 +++++
 6 files changed, 78 insertions(+), 12 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 22+ messages in thread
* [RFC PATCH 0/3] virtiofs,fuse: support per-file DAX
@ 2021-07-15  9:30 ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, bo.liu, virtualization

This patchset adds support of per-file DAX for virtiofs, which is
inspired by Ira Weiny's work on ext4[1] and xfs[2].

Currently virtiofs (in guest kernel) accepts per-file DAX flag from FUSE
server (in host). 

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Any comment is welcome. :)


[1] commit 9cb20f94afcd ("fs/ext4: Make DAX mount option a tri-state")
[2] commit 02beb2686ff9 ("fs/xfs: Make DAX mount option a tri-state")

Jeffle Xu (3):
  fuse: add fuse_should_enable_dax() helper
  fuse: Make DAX mount option a tri-state
  fuse: add per-file DAX flag

 fs/fuse/dax.c             | 43 +++++++++++++++++++++++++++++++++++++--
 fs/fuse/file.c            |  4 ++--
 fs/fuse/fuse_i.h          | 16 +++++++++++----
 fs/fuse/inode.c           |  6 ++++--
 fs/fuse/virtio_fs.c       | 16 +++++++++++++--
 include/uapi/linux/fuse.h |  5 +++++
 6 files changed, 78 insertions(+), 12 deletions(-)

-- 
2.27.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [RFC PATCH 1/3] fuse: add fuse_should_enable_dax() helper
  2021-07-15  9:30 ` Jeffle Xu
@ 2021-07-15  9:30   ` Jeffle Xu
  -1 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, virtualization, bo.liu

This is in prep for following per-file DAX checking.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 0e5407f48e6a..97b8bd09baa3 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1336,11 +1336,19 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 	.invalidatepage	= noop_invalidatepage,
 };
 
-void fuse_dax_inode_init(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 
 	if (!fc->dax)
+		return false;
+ 
+	return true;
+}
+
+void fuse_dax_inode_init(struct inode *inode)
+{
+	if (!fuse_should_enable_dax(inode))
 		return;
 
 	inode->i_flags |= S_DAX;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [RFC PATCH 1/3] fuse: add fuse_should_enable_dax() helper
@ 2021-07-15  9:30   ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, bo.liu, virtualization

This is in prep for following per-file DAX checking.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 0e5407f48e6a..97b8bd09baa3 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1336,11 +1336,19 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 	.invalidatepage	= noop_invalidatepage,
 };
 
-void fuse_dax_inode_init(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 
 	if (!fc->dax)
+		return false;
+ 
+	return true;
+}
+
+void fuse_dax_inode_init(struct inode *inode)
+{
+	if (!fuse_should_enable_dax(inode))
 		return;
 
 	inode->i_flags |= S_DAX;
-- 
2.27.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [RFC PATCH 2/3] fuse: Make DAX mount option a tri-state
  2021-07-15  9:30 ` Jeffle Xu
@ 2021-07-15  9:30   ` Jeffle Xu
  -1 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, virtualization, bo.liu

We add 'always', 'never', and 'inode' (default). '-o dax' continues to
operate the same which is equivalent to 'always'.

By the time this patch is applied, 'inode' mode is actually equal to
'always' mode, before the per-file DAX flag is introduced in the
following patch.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c       | 13 ++++++++++++-
 fs/fuse/fuse_i.h    | 11 +++++++++--
 fs/fuse/inode.c     |  2 +-
 fs/fuse/virtio_fs.c | 16 ++++++++++++++--
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 97b8bd09baa3..4873d764cb66 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -70,6 +70,9 @@ struct fuse_inode_dax {
 };
 
 struct fuse_conn_dax {
+	/** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/
+	unsigned int mode;
+
 	/* DAX device */
 	struct dax_device *dev;
 
@@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
 	return ret;
 }
 
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+			struct dax_device *dax_dev)
 {
 	struct fuse_conn_dax *fcd;
 	int err;
@@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
 		return -ENOMEM;
 
 	spin_lock_init(&fcd->lock);
+	fcd->mode = mode;
 	fcd->dev = dax_dev;
 	err = fuse_dax_mem_range_init(fcd);
 	if (err) {
@@ -1339,10 +1344,16 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 static bool fuse_should_enable_dax(struct inode *inode)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
+	unsigned int mode;
 
 	if (!fc->dax)
 		return false;
  
+	mode = fc->dax->mode;
+
+	if (mode == FUSE_DAX_MOUNT_NEVER)
+		return false;
+
 	return true;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 07829ce78695..f29018323845 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -487,6 +487,12 @@ struct fuse_dev {
 	struct list_head entry;
 };
 
+enum {
+	FUSE_DAX_MOUNT_INODE,
+	FUSE_DAX_MOUNT_ALWAYS,
+	FUSE_DAX_MOUNT_NEVER,
+};
+
 struct fuse_fs_context {
 	int fd;
 	unsigned int rootmode;
@@ -503,7 +509,7 @@ struct fuse_fs_context {
 	bool no_control:1;
 	bool no_force_umount:1;
 	bool legacy_opts_show:1;
-	bool dax:1;
+	unsigned int dax;
 	unsigned int max_read;
 	unsigned int blksize;
 	const char *subtype;
@@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to);
 ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from);
 int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma);
 int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end);
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev);
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+			struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
 void fuse_dax_inode_init(struct inode *inode);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b9beb39a4a18..f6b46395edb2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 	sb->s_subtype = ctx->subtype;
 	ctx->subtype = NULL;
 	if (IS_ENABLED(CONFIG_FUSE_DAX)) {
-		err = fuse_dax_conn_alloc(fc, ctx->dax_dev);
+		err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev);
 		if (err)
 			goto err;
 	}
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 8f52cdaa8445..561f711d1945 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -88,12 +88,21 @@ struct virtio_fs_req_work {
 static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
 				 struct fuse_req *req, bool in_flight);
 
+static const struct constant_table dax_param_enums[] = {
+	{"inode",	FUSE_DAX_MOUNT_INODE },
+	{"always",	FUSE_DAX_MOUNT_ALWAYS },
+	{"never",	FUSE_DAX_MOUNT_NEVER },
+	{}
+};
+
 enum {
 	OPT_DAX,
+	OPT_DAX_ENUM,
 };
 
 static const struct fs_parameter_spec virtio_fs_parameters[] = {
 	fsparam_flag("dax", OPT_DAX),
+	fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums),
 	{}
 };
 
@@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc,
 
 	switch (opt) {
 	case OPT_DAX:
-		ctx->dax = 1;
+		ctx->dax = FUSE_DAX_MOUNT_ALWAYS;
+		break;
+	case OPT_DAX_ENUM:
+		ctx->dax = result.uint_32;
 		break;
 	default:
 		return -EINVAL;
@@ -1326,7 +1338,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
 
 	/* virtiofs allocates and installs its own fuse devices */
 	ctx->fudptr = NULL;
-	if (ctx->dax) {
+	if (ctx->dax != FUSE_DAX_MOUNT_NEVER) {
 		if (!fs->dax_dev) {
 			err = -EINVAL;
 			pr_err("virtio-fs: dax can't be enabled as filesystem"
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [RFC PATCH 2/3] fuse: Make DAX mount option a tri-state
@ 2021-07-15  9:30   ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, bo.liu, virtualization

We add 'always', 'never', and 'inode' (default). '-o dax' continues to
operate the same which is equivalent to 'always'.

By the time this patch is applied, 'inode' mode is actually equal to
'always' mode, before the per-file DAX flag is introduced in the
following patch.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c       | 13 ++++++++++++-
 fs/fuse/fuse_i.h    | 11 +++++++++--
 fs/fuse/inode.c     |  2 +-
 fs/fuse/virtio_fs.c | 16 ++++++++++++++--
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 97b8bd09baa3..4873d764cb66 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -70,6 +70,9 @@ struct fuse_inode_dax {
 };
 
 struct fuse_conn_dax {
+	/** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/
+	unsigned int mode;
+
 	/* DAX device */
 	struct dax_device *dev;
 
@@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd)
 	return ret;
 }
 
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+			struct dax_device *dax_dev)
 {
 	struct fuse_conn_dax *fcd;
 	int err;
@@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev)
 		return -ENOMEM;
 
 	spin_lock_init(&fcd->lock);
+	fcd->mode = mode;
 	fcd->dev = dax_dev;
 	err = fuse_dax_mem_range_init(fcd);
 	if (err) {
@@ -1339,10 +1344,16 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 static bool fuse_should_enable_dax(struct inode *inode)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
+	unsigned int mode;
 
 	if (!fc->dax)
 		return false;
  
+	mode = fc->dax->mode;
+
+	if (mode == FUSE_DAX_MOUNT_NEVER)
+		return false;
+
 	return true;
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 07829ce78695..f29018323845 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -487,6 +487,12 @@ struct fuse_dev {
 	struct list_head entry;
 };
 
+enum {
+	FUSE_DAX_MOUNT_INODE,
+	FUSE_DAX_MOUNT_ALWAYS,
+	FUSE_DAX_MOUNT_NEVER,
+};
+
 struct fuse_fs_context {
 	int fd;
 	unsigned int rootmode;
@@ -503,7 +509,7 @@ struct fuse_fs_context {
 	bool no_control:1;
 	bool no_force_umount:1;
 	bool legacy_opts_show:1;
-	bool dax:1;
+	unsigned int dax;
 	unsigned int max_read;
 	unsigned int blksize;
 	const char *subtype;
@@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to);
 ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from);
 int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma);
 int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end);
-int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev);
+int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
+			struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
 void fuse_dax_inode_init(struct inode *inode);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b9beb39a4a18..f6b46395edb2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 	sb->s_subtype = ctx->subtype;
 	ctx->subtype = NULL;
 	if (IS_ENABLED(CONFIG_FUSE_DAX)) {
-		err = fuse_dax_conn_alloc(fc, ctx->dax_dev);
+		err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev);
 		if (err)
 			goto err;
 	}
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 8f52cdaa8445..561f711d1945 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -88,12 +88,21 @@ struct virtio_fs_req_work {
 static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,
 				 struct fuse_req *req, bool in_flight);
 
+static const struct constant_table dax_param_enums[] = {
+	{"inode",	FUSE_DAX_MOUNT_INODE },
+	{"always",	FUSE_DAX_MOUNT_ALWAYS },
+	{"never",	FUSE_DAX_MOUNT_NEVER },
+	{}
+};
+
 enum {
 	OPT_DAX,
+	OPT_DAX_ENUM,
 };
 
 static const struct fs_parameter_spec virtio_fs_parameters[] = {
 	fsparam_flag("dax", OPT_DAX),
+	fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums),
 	{}
 };
 
@@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc,
 
 	switch (opt) {
 	case OPT_DAX:
-		ctx->dax = 1;
+		ctx->dax = FUSE_DAX_MOUNT_ALWAYS;
+		break;
+	case OPT_DAX_ENUM:
+		ctx->dax = result.uint_32;
 		break;
 	default:
 		return -EINVAL;
@@ -1326,7 +1338,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
 
 	/* virtiofs allocates and installs its own fuse devices */
 	ctx->fudptr = NULL;
-	if (ctx->dax) {
+	if (ctx->dax != FUSE_DAX_MOUNT_NEVER) {
 		if (!fs->dax_dev) {
 			err = -EINVAL;
 			pr_err("virtio-fs: dax can't be enabled as filesystem"
-- 
2.27.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-15  9:30 ` Jeffle Xu
@ 2021-07-15  9:30   ` Jeffle Xu
  -1 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, virtualization, bo.liu

Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
this file.

When the per-file DAX flag changes for an *opened* file, the state of
the file won't be updated until this file is closed and reopened later.

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c             | 28 ++++++++++++++++++++++++----
 fs/fuse/file.c            |  4 ++--
 fs/fuse/fuse_i.h          |  5 +++--
 fs/fuse/inode.c           |  4 +++-
 include/uapi/linux/fuse.h |  5 +++++
 5 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 4873d764cb66..ed5a430364bb 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1341,7 +1341,7 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 	.invalidatepage	= noop_invalidatepage,
 };
 
-static bool fuse_should_enable_dax(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	unsigned int mode;
@@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
 	if (mode == FUSE_DAX_MOUNT_NEVER)
 		return false;
 
-	return true;
+	if (mode == FUSE_DAX_MOUNT_ALWAYS)
+		return true;
+
+	WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
+	return flags & FUSE_ATTR_DAX;
 }
 
-void fuse_dax_inode_init(struct inode *inode)
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
 {
-	if (!fuse_should_enable_dax(inode))
+	if (!fuse_should_enable_dax(inode, flags))
 		return;
 
 	inode->i_flags |= S_DAX;
 	inode->i_data.a_ops = &fuse_dax_file_aops;
 }
 
+void fuse_update_dax(struct inode *inode, unsigned int flags)
+{
+	bool oldstate, newstate;
+	struct fuse_conn *fc = get_fuse_conn(inode);
+
+	if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
+	    fc->dax->mode != FUSE_DAX_MOUNT_INODE)
+		return;
+
+	oldstate = IS_DAX(inode);
+	newstate = flags & FUSE_ATTR_DAX;
+
+	if (oldstate != newstate)
+		d_mark_dontcache(inode);
+}
+
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
 {
 	if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 97f860cfc195..cf42af492146 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3142,7 +3142,7 @@ static const struct address_space_operations fuse_file_aops  = {
 	.write_end	= fuse_write_end,
 };
 
-void fuse_init_file_inode(struct inode *inode)
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
@@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
 	fi->writepages = RB_ROOT;
 
 	if (IS_ENABLED(CONFIG_FUSE_DAX))
-		fuse_dax_inode_init(inode);
+		fuse_dax_inode_init(inode, attr->flags);
 }
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f29018323845..b0ecfffd0c7d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
 /**
  * Initialize file operations on a regular file
  */
-void fuse_init_file_inode(struct inode *inode);
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
 
 /**
  * Initialize inode operations on regular files and special files
@@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
 			struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
-void fuse_dax_inode_init(struct inode *inode);
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
 void fuse_dax_inode_cleanup(struct inode *inode);
+void fuse_update_dax(struct inode *inode, unsigned int flags);
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment);
 void fuse_dax_cancel_work(struct fuse_conn *fc);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f6b46395edb2..47ebb1a394d2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 		if (inval)
 			invalidate_inode_pages2(inode->i_mapping);
 	}
+
+	fuse_update_dax(inode, attr->flags);
 }
 
 static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
@@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
 	inode->i_ctime.tv_nsec = attr->ctimensec;
 	if (S_ISREG(inode->i_mode)) {
 		fuse_init_common(inode);
-		fuse_init_file_inode(inode);
+		fuse_init_file_inode(inode, attr);
 	} else if (S_ISDIR(inode->i_mode))
 		fuse_init_dir(inode);
 	else if (S_ISLNK(inode->i_mode))
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 36ed092227fa..9ee088ddbe2a 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -184,6 +184,9 @@
  *
  *  7.34
  *  - add FUSE_SYNCFS
+ *
+ *  7.35
+ *  - add FUSE_ATTR_DAX
  */
 
 #ifndef _LINUX_FUSE_H
@@ -449,8 +452,10 @@ struct fuse_file_lock {
  * fuse_attr flags
  *
  * FUSE_ATTR_SUBMOUNT: Object is a submount root
+ * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
  */
 #define FUSE_ATTR_SUBMOUNT      (1 << 0)
+#define FUSE_ATTR_DAX      	(1 << 1)
 
 /**
  * Open flags
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [RFC PATCH 3/3] fuse: add per-file DAX flag
@ 2021-07-15  9:30   ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-15  9:30 UTC (permalink / raw)
  To: vgoyal, stefanha, miklos; +Cc: linux-fsdevel, bo.liu, virtualization

Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
this file.

When the per-file DAX flag changes for an *opened* file, the state of
the file won't be updated until this file is closed and reopened later.

Currently it is not implemented yet to change per-file DAX flag inside
guest kernel, e.g., by chattr(1).

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/dax.c             | 28 ++++++++++++++++++++++++----
 fs/fuse/file.c            |  4 ++--
 fs/fuse/fuse_i.h          |  5 +++--
 fs/fuse/inode.c           |  4 +++-
 include/uapi/linux/fuse.h |  5 +++++
 5 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
index 4873d764cb66..ed5a430364bb 100644
--- a/fs/fuse/dax.c
+++ b/fs/fuse/dax.c
@@ -1341,7 +1341,7 @@ static const struct address_space_operations fuse_dax_file_aops  = {
 	.invalidatepage	= noop_invalidatepage,
 };
 
-static bool fuse_should_enable_dax(struct inode *inode)
+static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	unsigned int mode;
@@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
 	if (mode == FUSE_DAX_MOUNT_NEVER)
 		return false;
 
-	return true;
+	if (mode == FUSE_DAX_MOUNT_ALWAYS)
+		return true;
+
+	WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
+	return flags & FUSE_ATTR_DAX;
 }
 
-void fuse_dax_inode_init(struct inode *inode)
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
 {
-	if (!fuse_should_enable_dax(inode))
+	if (!fuse_should_enable_dax(inode, flags))
 		return;
 
 	inode->i_flags |= S_DAX;
 	inode->i_data.a_ops = &fuse_dax_file_aops;
 }
 
+void fuse_update_dax(struct inode *inode, unsigned int flags)
+{
+	bool oldstate, newstate;
+	struct fuse_conn *fc = get_fuse_conn(inode);
+
+	if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
+	    fc->dax->mode != FUSE_DAX_MOUNT_INODE)
+		return;
+
+	oldstate = IS_DAX(inode);
+	newstate = flags & FUSE_ATTR_DAX;
+
+	if (oldstate != newstate)
+		d_mark_dontcache(inode);
+}
+
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
 {
 	if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 97f860cfc195..cf42af492146 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3142,7 +3142,7 @@ static const struct address_space_operations fuse_file_aops  = {
 	.write_end	= fuse_write_end,
 };
 
-void fuse_init_file_inode(struct inode *inode)
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
@@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
 	fi->writepages = RB_ROOT;
 
 	if (IS_ENABLED(CONFIG_FUSE_DAX))
-		fuse_dax_inode_init(inode);
+		fuse_dax_inode_init(inode, attr->flags);
 }
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index f29018323845..b0ecfffd0c7d 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
 /**
  * Initialize file operations on a regular file
  */
-void fuse_init_file_inode(struct inode *inode);
+void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
 
 /**
  * Initialize inode operations on regular files and special files
@@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
 			struct dax_device *dax_dev);
 void fuse_dax_conn_free(struct fuse_conn *fc);
 bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
-void fuse_dax_inode_init(struct inode *inode);
+void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
 void fuse_dax_inode_cleanup(struct inode *inode);
+void fuse_update_dax(struct inode *inode, unsigned int flags);
 bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment);
 void fuse_dax_cancel_work(struct fuse_conn *fc);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f6b46395edb2..47ebb1a394d2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
 		if (inval)
 			invalidate_inode_pages2(inode->i_mapping);
 	}
+
+	fuse_update_dax(inode, attr->flags);
 }
 
 static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
@@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
 	inode->i_ctime.tv_nsec = attr->ctimensec;
 	if (S_ISREG(inode->i_mode)) {
 		fuse_init_common(inode);
-		fuse_init_file_inode(inode);
+		fuse_init_file_inode(inode, attr);
 	} else if (S_ISDIR(inode->i_mode))
 		fuse_init_dir(inode);
 	else if (S_ISLNK(inode->i_mode))
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 36ed092227fa..9ee088ddbe2a 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -184,6 +184,9 @@
  *
  *  7.34
  *  - add FUSE_SYNCFS
+ *
+ *  7.35
+ *  - add FUSE_ATTR_DAX
  */
 
 #ifndef _LINUX_FUSE_H
@@ -449,8 +452,10 @@ struct fuse_file_lock {
  * fuse_attr flags
  *
  * FUSE_ATTR_SUBMOUNT: Object is a submount root
+ * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
  */
 #define FUSE_ATTR_SUBMOUNT      (1 << 0)
+#define FUSE_ATTR_DAX      	(1 << 1)
 
 /**
  * Open flags
-- 
2.27.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-15  9:30   ` Jeffle Xu
  (?)
@ 2021-07-15 15:13   ` kernel test robot
  2021-07-16  1:09     ` JeffleXu
  -1 siblings, 1 reply; 22+ messages in thread
From: kernel test robot @ 2021-07-15 15:13 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]

Hi Jeffle,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on fuse/for-next]
[also build test ERROR on v5.14-rc1 next-20210715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jeffle-Xu/virtiofs-fuse-support-per-file-DAX/20210715-173102
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0e8ed9f530357cba01ae4bcaf4284a2da59aef62
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeffle-Xu/virtiofs-fuse-support-per-file-DAX/20210715-173102
        git checkout 0e8ed9f530357cba01ae4bcaf4284a2da59aef62
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   nds32le-linux-ld: fs/fuse/inode.o: in function `fuse_change_attributes':
   inode.c:(.text+0x1f98): undefined reference to `fuse_update_dax'
>> nds32le-linux-ld: inode.c:(.text+0x1f9c): undefined reference to `fuse_update_dax'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 10807 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-15  9:30   ` Jeffle Xu
  (?)
  (?)
@ 2021-07-15 15:24   ` kernel test robot
  -1 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-07-15 15:24 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

Hi Jeffle,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on fuse/for-next]
[also build test ERROR on v5.14-rc1 next-20210715]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jeffle-Xu/virtiofs-fuse-support-per-file-DAX/20210715-173102
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
config: arm64-randconfig-c003-20210715 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0e8ed9f530357cba01ae4bcaf4284a2da59aef62
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeffle-Xu/virtiofs-fuse-support-per-file-DAX/20210715-173102
        git checkout 0e8ed9f530357cba01ae4bcaf4284a2da59aef62
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "fuse_update_dax" [fs/fuse/fuse.ko] undefined!

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28458 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-15  9:30   ` Jeffle Xu
                     ` (2 preceding siblings ...)
  (?)
@ 2021-07-16  0:40   ` Liu Bo
  2021-07-16  0:51       ` Vivek Goyal
  -1 siblings, 1 reply; 22+ messages in thread
From: Liu Bo @ 2021-07-16  0:40 UTC (permalink / raw)
  To: Jeffle Xu; +Cc: vgoyal, stefanha, miklos, linux-fsdevel, virtualization

On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> this file.
> 
> When the per-file DAX flag changes for an *opened* file, the state of
> the file won't be updated until this file is closed and reopened later.
> 
> Currently it is not implemented yet to change per-file DAX flag inside
> guest kernel, e.g., by chattr(1).

Thanks for the patch, it looks good to me.

I think it's a good starting point, what I'd like to discuss here is
whether we're going to let chattr to toggle the dax flag.

My usecase is like, on the fuse server side, if a file is marked as
DAX, then it won't change any more.  So this 'fuse_attr.flags' works
for me at least.

thanks,
liubo

> 
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
>  fs/fuse/dax.c             | 28 ++++++++++++++++++++++++----
>  fs/fuse/file.c            |  4 ++--
>  fs/fuse/fuse_i.h          |  5 +++--
>  fs/fuse/inode.c           |  4 +++-
>  include/uapi/linux/fuse.h |  5 +++++
>  5 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 4873d764cb66..ed5a430364bb 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -1341,7 +1341,7 @@ static const struct address_space_operations fuse_dax_file_aops  = {
>  	.invalidatepage	= noop_invalidatepage,
>  };
>  
> -static bool fuse_should_enable_dax(struct inode *inode)
> +static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
>  {
>  	struct fuse_conn *fc = get_fuse_conn(inode);
>  	unsigned int mode;
> @@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
>  	if (mode == FUSE_DAX_MOUNT_NEVER)
>  		return false;
>  
> -	return true;
> +	if (mode == FUSE_DAX_MOUNT_ALWAYS)
> +		return true;
> +
> +	WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
> +	return flags & FUSE_ATTR_DAX;
>  }
>  
> -void fuse_dax_inode_init(struct inode *inode)
> +void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
>  {
> -	if (!fuse_should_enable_dax(inode))
> +	if (!fuse_should_enable_dax(inode, flags))
>  		return;
>  
>  	inode->i_flags |= S_DAX;
>  	inode->i_data.a_ops = &fuse_dax_file_aops;
>  }
>  
> +void fuse_update_dax(struct inode *inode, unsigned int flags)
> +{
> +	bool oldstate, newstate;
> +	struct fuse_conn *fc = get_fuse_conn(inode);
> +
> +	if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
> +	    fc->dax->mode != FUSE_DAX_MOUNT_INODE)
> +		return;
> +
> +	oldstate = IS_DAX(inode);
> +	newstate = flags & FUSE_ATTR_DAX;
> +
> +	if (oldstate != newstate)
> +		d_mark_dontcache(inode);
> +}
> +
>  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
>  {
>  	if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 97f860cfc195..cf42af492146 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -3142,7 +3142,7 @@ static const struct address_space_operations fuse_file_aops  = {
>  	.write_end	= fuse_write_end,
>  };
>  
> -void fuse_init_file_inode(struct inode *inode)
> +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
>  {
>  	struct fuse_inode *fi = get_fuse_inode(inode);
>  
> @@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
>  	fi->writepages = RB_ROOT;
>  
>  	if (IS_ENABLED(CONFIG_FUSE_DAX))
> -		fuse_dax_inode_init(inode);
> +		fuse_dax_inode_init(inode, attr->flags);
>  }
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index f29018323845..b0ecfffd0c7d 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
>  /**
>   * Initialize file operations on a regular file
>   */
> -void fuse_init_file_inode(struct inode *inode);
> +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
>  
>  /**
>   * Initialize inode operations on regular files and special files
> @@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
>  			struct dax_device *dax_dev);
>  void fuse_dax_conn_free(struct fuse_conn *fc);
>  bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
> -void fuse_dax_inode_init(struct inode *inode);
> +void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
>  void fuse_dax_inode_cleanup(struct inode *inode);
> +void fuse_update_dax(struct inode *inode, unsigned int flags);
>  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment);
>  void fuse_dax_cancel_work(struct fuse_conn *fc);
>  
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index f6b46395edb2..47ebb1a394d2 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
>  		if (inval)
>  			invalidate_inode_pages2(inode->i_mapping);
>  	}
> +
> +	fuse_update_dax(inode, attr->flags);
>  }
>  
>  static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
> @@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
>  	inode->i_ctime.tv_nsec = attr->ctimensec;
>  	if (S_ISREG(inode->i_mode)) {
>  		fuse_init_common(inode);
> -		fuse_init_file_inode(inode);
> +		fuse_init_file_inode(inode, attr);
>  	} else if (S_ISDIR(inode->i_mode))
>  		fuse_init_dir(inode);
>  	else if (S_ISLNK(inode->i_mode))
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 36ed092227fa..9ee088ddbe2a 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -184,6 +184,9 @@
>   *
>   *  7.34
>   *  - add FUSE_SYNCFS
> + *
> + *  7.35
> + *  - add FUSE_ATTR_DAX
>   */
>  
>  #ifndef _LINUX_FUSE_H
> @@ -449,8 +452,10 @@ struct fuse_file_lock {
>   * fuse_attr flags
>   *
>   * FUSE_ATTR_SUBMOUNT: Object is a submount root
> + * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
>   */
>  #define FUSE_ATTR_SUBMOUNT      (1 << 0)
> +#define FUSE_ATTR_DAX      	(1 << 1)
>  
>  /**
>   * Open flags
> -- 
> 2.27.0

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-16  0:40   ` Liu Bo
@ 2021-07-16  0:51       ` Vivek Goyal
  0 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2021-07-16  0:51 UTC (permalink / raw)
  To: Liu Bo; +Cc: Jeffle Xu, stefanha, miklos, linux-fsdevel, virtualization

On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> > Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> > this file.
> > 
> > When the per-file DAX flag changes for an *opened* file, the state of
> > the file won't be updated until this file is closed and reopened later.
> > 
> > Currently it is not implemented yet to change per-file DAX flag inside
> > guest kernel, e.g., by chattr(1).
> 
> Thanks for the patch, it looks good to me.
> 
> I think it's a good starting point, what I'd like to discuss here is
> whether we're going to let chattr to toggle the dax flag.

I have the same question. Why not take chattr approach as taken
by ext4/xfs as well.

Vivek

> 
> My usecase is like, on the fuse server side, if a file is marked as
> DAX, then it won't change any more.  So this 'fuse_attr.flags' works
> for me at least.
> 
> thanks,
> liubo
> 
> > 
> > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> > ---
> >  fs/fuse/dax.c             | 28 ++++++++++++++++++++++++----
> >  fs/fuse/file.c            |  4 ++--
> >  fs/fuse/fuse_i.h          |  5 +++--
> >  fs/fuse/inode.c           |  4 +++-
> >  include/uapi/linux/fuse.h |  5 +++++
> >  5 files changed, 37 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > index 4873d764cb66..ed5a430364bb 100644
> > --- a/fs/fuse/dax.c
> > +++ b/fs/fuse/dax.c
> > @@ -1341,7 +1341,7 @@ static const struct address_space_operations fuse_dax_file_aops  = {
> >  	.invalidatepage	= noop_invalidatepage,
> >  };
> >  
> > -static bool fuse_should_enable_dax(struct inode *inode)
> > +static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
> >  {
> >  	struct fuse_conn *fc = get_fuse_conn(inode);
> >  	unsigned int mode;
> > @@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
> >  	if (mode == FUSE_DAX_MOUNT_NEVER)
> >  		return false;
> >  
> > -	return true;
> > +	if (mode == FUSE_DAX_MOUNT_ALWAYS)
> > +		return true;
> > +
> > +	WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
> > +	return flags & FUSE_ATTR_DAX;
> >  }
> >  
> > -void fuse_dax_inode_init(struct inode *inode)
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
> >  {
> > -	if (!fuse_should_enable_dax(inode))
> > +	if (!fuse_should_enable_dax(inode, flags))
> >  		return;
> >  
> >  	inode->i_flags |= S_DAX;
> >  	inode->i_data.a_ops = &fuse_dax_file_aops;
> >  }
> >  
> > +void fuse_update_dax(struct inode *inode, unsigned int flags)
> > +{
> > +	bool oldstate, newstate;
> > +	struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > +	if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
> > +	    fc->dax->mode != FUSE_DAX_MOUNT_INODE)
> > +		return;
> > +
> > +	oldstate = IS_DAX(inode);
> > +	newstate = flags & FUSE_ATTR_DAX;
> > +
> > +	if (oldstate != newstate)
> > +		d_mark_dontcache(inode);
> > +}
> > +
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
> >  {
> >  	if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 97f860cfc195..cf42af492146 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3142,7 +3142,7 @@ static const struct address_space_operations fuse_file_aops  = {
> >  	.write_end	= fuse_write_end,
> >  };
> >  
> > -void fuse_init_file_inode(struct inode *inode)
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
> >  {
> >  	struct fuse_inode *fi = get_fuse_inode(inode);
> >  
> > @@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
> >  	fi->writepages = RB_ROOT;
> >  
> >  	if (IS_ENABLED(CONFIG_FUSE_DAX))
> > -		fuse_dax_inode_init(inode);
> > +		fuse_dax_inode_init(inode, attr->flags);
> >  }
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index f29018323845..b0ecfffd0c7d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
> >  /**
> >   * Initialize file operations on a regular file
> >   */
> > -void fuse_init_file_inode(struct inode *inode);
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
> >  
> >  /**
> >   * Initialize inode operations on regular files and special files
> > @@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
> >  			struct dax_device *dax_dev);
> >  void fuse_dax_conn_free(struct fuse_conn *fc);
> >  bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
> > -void fuse_dax_inode_init(struct inode *inode);
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
> >  void fuse_dax_inode_cleanup(struct inode *inode);
> > +void fuse_update_dax(struct inode *inode, unsigned int flags);
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment);
> >  void fuse_dax_cancel_work(struct fuse_conn *fc);
> >  
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index f6b46395edb2..47ebb1a394d2 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
> >  		if (inval)
> >  			invalidate_inode_pages2(inode->i_mapping);
> >  	}
> > +
> > +	fuse_update_dax(inode, attr->flags);
> >  }
> >  
> >  static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
> > @@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
> >  	inode->i_ctime.tv_nsec = attr->ctimensec;
> >  	if (S_ISREG(inode->i_mode)) {
> >  		fuse_init_common(inode);
> > -		fuse_init_file_inode(inode);
> > +		fuse_init_file_inode(inode, attr);
> >  	} else if (S_ISDIR(inode->i_mode))
> >  		fuse_init_dir(inode);
> >  	else if (S_ISLNK(inode->i_mode))
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 36ed092227fa..9ee088ddbe2a 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -184,6 +184,9 @@
> >   *
> >   *  7.34
> >   *  - add FUSE_SYNCFS
> > + *
> > + *  7.35
> > + *  - add FUSE_ATTR_DAX
> >   */
> >  
> >  #ifndef _LINUX_FUSE_H
> > @@ -449,8 +452,10 @@ struct fuse_file_lock {
> >   * fuse_attr flags
> >   *
> >   * FUSE_ATTR_SUBMOUNT: Object is a submount root
> > + * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
> >   */
> >  #define FUSE_ATTR_SUBMOUNT      (1 << 0)
> > +#define FUSE_ATTR_DAX      	(1 << 1)
> >  
> >  /**
> >   * Open flags
> > -- 
> > 2.27.0
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
@ 2021-07-16  0:51       ` Vivek Goyal
  0 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2021-07-16  0:51 UTC (permalink / raw)
  To: Liu Bo; +Cc: linux-fsdevel, virtualization, stefanha, miklos

On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> > Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> > this file.
> > 
> > When the per-file DAX flag changes for an *opened* file, the state of
> > the file won't be updated until this file is closed and reopened later.
> > 
> > Currently it is not implemented yet to change per-file DAX flag inside
> > guest kernel, e.g., by chattr(1).
> 
> Thanks for the patch, it looks good to me.
> 
> I think it's a good starting point, what I'd like to discuss here is
> whether we're going to let chattr to toggle the dax flag.

I have the same question. Why not take chattr approach as taken
by ext4/xfs as well.

Vivek

> 
> My usecase is like, on the fuse server side, if a file is marked as
> DAX, then it won't change any more.  So this 'fuse_attr.flags' works
> for me at least.
> 
> thanks,
> liubo
> 
> > 
> > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> > ---
> >  fs/fuse/dax.c             | 28 ++++++++++++++++++++++++----
> >  fs/fuse/file.c            |  4 ++--
> >  fs/fuse/fuse_i.h          |  5 +++--
> >  fs/fuse/inode.c           |  4 +++-
> >  include/uapi/linux/fuse.h |  5 +++++
> >  5 files changed, 37 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > index 4873d764cb66..ed5a430364bb 100644
> > --- a/fs/fuse/dax.c
> > +++ b/fs/fuse/dax.c
> > @@ -1341,7 +1341,7 @@ static const struct address_space_operations fuse_dax_file_aops  = {
> >  	.invalidatepage	= noop_invalidatepage,
> >  };
> >  
> > -static bool fuse_should_enable_dax(struct inode *inode)
> > +static bool fuse_should_enable_dax(struct inode *inode, unsigned int flags)
> >  {
> >  	struct fuse_conn *fc = get_fuse_conn(inode);
> >  	unsigned int mode;
> > @@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct inode *inode)
> >  	if (mode == FUSE_DAX_MOUNT_NEVER)
> >  		return false;
> >  
> > -	return true;
> > +	if (mode == FUSE_DAX_MOUNT_ALWAYS)
> > +		return true;
> > +
> > +	WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
> > +	return flags & FUSE_ATTR_DAX;
> >  }
> >  
> > -void fuse_dax_inode_init(struct inode *inode)
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
> >  {
> > -	if (!fuse_should_enable_dax(inode))
> > +	if (!fuse_should_enable_dax(inode, flags))
> >  		return;
> >  
> >  	inode->i_flags |= S_DAX;
> >  	inode->i_data.a_ops = &fuse_dax_file_aops;
> >  }
> >  
> > +void fuse_update_dax(struct inode *inode, unsigned int flags)
> > +{
> > +	bool oldstate, newstate;
> > +	struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > +	if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
> > +	    fc->dax->mode != FUSE_DAX_MOUNT_INODE)
> > +		return;
> > +
> > +	oldstate = IS_DAX(inode);
> > +	newstate = flags & FUSE_ATTR_DAX;
> > +
> > +	if (oldstate != newstate)
> > +		d_mark_dontcache(inode);
> > +}
> > +
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment)
> >  {
> >  	if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 97f860cfc195..cf42af492146 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3142,7 +3142,7 @@ static const struct address_space_operations fuse_file_aops  = {
> >  	.write_end	= fuse_write_end,
> >  };
> >  
> > -void fuse_init_file_inode(struct inode *inode)
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr)
> >  {
> >  	struct fuse_inode *fi = get_fuse_inode(inode);
> >  
> > @@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
> >  	fi->writepages = RB_ROOT;
> >  
> >  	if (IS_ENABLED(CONFIG_FUSE_DAX))
> > -		fuse_dax_inode_init(inode);
> > +		fuse_dax_inode_init(inode, attr->flags);
> >  }
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index f29018323845..b0ecfffd0c7d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn *fc,
> >  /**
> >   * Initialize file operations on a regular file
> >   */
> > -void fuse_init_file_inode(struct inode *inode);
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr *attr);
> >  
> >  /**
> >   * Initialize inode operations on regular files and special files
> > @@ -1252,8 +1252,9 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode,
> >  			struct dax_device *dax_dev);
> >  void fuse_dax_conn_free(struct fuse_conn *fc);
> >  bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi);
> > -void fuse_dax_inode_init(struct inode *inode);
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
> >  void fuse_dax_inode_cleanup(struct inode *inode);
> > +void fuse_update_dax(struct inode *inode, unsigned int flags);
> >  bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment);
> >  void fuse_dax_cancel_work(struct fuse_conn *fc);
> >  
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index f6b46395edb2..47ebb1a394d2 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr,
> >  		if (inval)
> >  			invalidate_inode_pages2(inode->i_mapping);
> >  	}
> > +
> > +	fuse_update_dax(inode, attr->flags);
> >  }
> >  
> >  static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
> > @@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
> >  	inode->i_ctime.tv_nsec = attr->ctimensec;
> >  	if (S_ISREG(inode->i_mode)) {
> >  		fuse_init_common(inode);
> > -		fuse_init_file_inode(inode);
> > +		fuse_init_file_inode(inode, attr);
> >  	} else if (S_ISDIR(inode->i_mode))
> >  		fuse_init_dir(inode);
> >  	else if (S_ISLNK(inode->i_mode))
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 36ed092227fa..9ee088ddbe2a 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -184,6 +184,9 @@
> >   *
> >   *  7.34
> >   *  - add FUSE_SYNCFS
> > + *
> > + *  7.35
> > + *  - add FUSE_ATTR_DAX
> >   */
> >  
> >  #ifndef _LINUX_FUSE_H
> > @@ -449,8 +452,10 @@ struct fuse_file_lock {
> >   * fuse_attr flags
> >   *
> >   * FUSE_ATTR_SUBMOUNT: Object is a submount root
> > + * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
> >   */
> >  #define FUSE_ATTR_SUBMOUNT      (1 << 0)
> > +#define FUSE_ATTR_DAX      	(1 << 1)
> >  
> >  /**
> >   * Open flags
> > -- 
> > 2.27.0
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-15 15:13   ` kernel test robot
@ 2021-07-16  1:09     ` JeffleXu
  0 siblings, 0 replies; 22+ messages in thread
From: JeffleXu @ 2021-07-16  1:09 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]



On 7/15/21 11:13 PM, kernel test robot wrote:
> All errors (new ones prefixed by >>):
> 
>    nds32le-linux-ld: fs/fuse/inode.o: in function `fuse_change_attributes':
>    inode.c:(.text+0x1f98): undefined reference to `fuse_update_dax'
>>> nds32le-linux-ld: inode.c:(.text+0x1f9c): undefined reference to `fuse_update_dax'

It is because 'fuse_update_dax' is defined only when CONFIG_FUSE_DAX is
defined.

I will include the modification in the next version.

```
if (IS_ENABLED(CONFIG_FUSE_DAX))
	fuse_update_dax(...)
```

-- 
Thanks,
Jeffle

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-16  0:51       ` Vivek Goyal
@ 2021-07-16  1:18         ` JeffleXu
  -1 siblings, 0 replies; 22+ messages in thread
From: JeffleXu @ 2021-07-16  1:18 UTC (permalink / raw)
  To: Vivek Goyal, Liu Bo; +Cc: stefanha, miklos, linux-fsdevel, virtualization



On 7/16/21 8:51 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
>> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
>>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
>>> this file.
>>>
>>> When the per-file DAX flag changes for an *opened* file, the state of
>>> the file won't be updated until this file is closed and reopened later.
>>>
>>> Currently it is not implemented yet to change per-file DAX flag inside
>>> guest kernel, e.g., by chattr(1).
>>
>> Thanks for the patch, it looks good to me.
>>
>> I think it's a good starting point, what I'd like to discuss here is
>> whether we're going to let chattr to toggle the dax flag.
> 
> I have the same question. Why not take chattr approach as taken
> by ext4/xfs as well.
> 
> Vivek

Thanks.

We can implement the chattr approach as ext4/xfs do, if we have this use
scenario. It's an RFC patch, and I want to collect more feedback as soon
as possible.

-- 
Thanks,
Jeffle

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
@ 2021-07-16  1:18         ` JeffleXu
  0 siblings, 0 replies; 22+ messages in thread
From: JeffleXu @ 2021-07-16  1:18 UTC (permalink / raw)
  To: Vivek Goyal, Liu Bo; +Cc: linux-fsdevel, virtualization, stefanha, miklos



On 7/16/21 8:51 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
>> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
>>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
>>> this file.
>>>
>>> When the per-file DAX flag changes for an *opened* file, the state of
>>> the file won't be updated until this file is closed and reopened later.
>>>
>>> Currently it is not implemented yet to change per-file DAX flag inside
>>> guest kernel, e.g., by chattr(1).
>>
>> Thanks for the patch, it looks good to me.
>>
>> I think it's a good starting point, what I'd like to discuss here is
>> whether we're going to let chattr to toggle the dax flag.
> 
> I have the same question. Why not take chattr approach as taken
> by ext4/xfs as well.
> 
> Vivek

Thanks.

We can implement the chattr approach as ext4/xfs do, if we have this use
scenario. It's an RFC patch, and I want to collect more feedback as soon
as possible.

-- 
Thanks,
Jeffle
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-16  1:18         ` JeffleXu
@ 2021-07-16  1:32           ` Vivek Goyal
  -1 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2021-07-16  1:32 UTC (permalink / raw)
  To: JeffleXu; +Cc: Liu Bo, stefanha, miklos, linux-fsdevel, virtualization

On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
> 
> 
> On 7/16/21 8:51 AM, Vivek Goyal wrote:
> > On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> >> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> >>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> >>> this file.
> >>>
> >>> When the per-file DAX flag changes for an *opened* file, the state of
> >>> the file won't be updated until this file is closed and reopened later.
> >>>
> >>> Currently it is not implemented yet to change per-file DAX flag inside
> >>> guest kernel, e.g., by chattr(1).
> >>
> >> Thanks for the patch, it looks good to me.
> >>
> >> I think it's a good starting point, what I'd like to discuss here is
> >> whether we're going to let chattr to toggle the dax flag.
> > 
> > I have the same question. Why not take chattr approach as taken
> > by ext4/xfs as well.
> > 
> > Vivek
> 
> Thanks.
> 
> We can implement the chattr approach as ext4/xfs do, if we have this use
> scenario. It's an RFC patch, and I want to collect more feedback as soon
> as possible.

I guess chattr approach will allow client (as well as server) to control
which files should be DAX. While this approach allows only server to
specify which files should use DAX. Given currently we let client
control whether to use dax or not (-o dax), it probably will make
sense to use chattr based approach?

I will look at the patches. Do you have a corresponding user space
implementation somewhere so that I can test it?

Vivek


^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
@ 2021-07-16  1:32           ` Vivek Goyal
  0 siblings, 0 replies; 22+ messages in thread
From: Vivek Goyal @ 2021-07-16  1:32 UTC (permalink / raw)
  To: JeffleXu; +Cc: linux-fsdevel, virtualization, Liu Bo, stefanha, miklos

On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
> 
> 
> On 7/16/21 8:51 AM, Vivek Goyal wrote:
> > On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
> >> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> >>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
> >>> this file.
> >>>
> >>> When the per-file DAX flag changes for an *opened* file, the state of
> >>> the file won't be updated until this file is closed and reopened later.
> >>>
> >>> Currently it is not implemented yet to change per-file DAX flag inside
> >>> guest kernel, e.g., by chattr(1).
> >>
> >> Thanks for the patch, it looks good to me.
> >>
> >> I think it's a good starting point, what I'd like to discuss here is
> >> whether we're going to let chattr to toggle the dax flag.
> > 
> > I have the same question. Why not take chattr approach as taken
> > by ext4/xfs as well.
> > 
> > Vivek
> 
> Thanks.
> 
> We can implement the chattr approach as ext4/xfs do, if we have this use
> scenario. It's an RFC patch, and I want to collect more feedback as soon
> as possible.

I guess chattr approach will allow client (as well as server) to control
which files should be DAX. While this approach allows only server to
specify which files should use DAX. Given currently we let client
control whether to use dax or not (-o dax), it probably will make
sense to use chattr based approach?

I will look at the patches. Do you have a corresponding user space
implementation somewhere so that I can test it?

Vivek

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH] virtiofsd: support per-file DAX
  2021-07-16  1:32           ` Vivek Goyal
@ 2021-07-16  1:52             ` Jeffle Xu
  -1 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-16  1:52 UTC (permalink / raw)
  To: vgoyal; +Cc: stefanha, miklos, linux-fsdevel, virtualization, bo.liu

An example implementation of supporting per-file DAX flag for virtiofsd,
where DAx is enabled for files larger than 1M size.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 contrib/virtiofsd/fuse_kernel.h   | 4 +++-
 contrib/virtiofsd/fuse_lowlevel.c | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
index d2b7ccf96b..9c476b7021 100644
--- a/contrib/virtiofsd/fuse_kernel.h
+++ b/contrib/virtiofsd/fuse_kernel.h
@@ -165,6 +165,8 @@
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
 
+#define FUSE_ATTR_DAX  (1 << 1)
+
 /* Make sure all structures are padded to 64bit boundary, so 32bit
    userspace works under 64bit kernels */
 
@@ -184,7 +186,7 @@ struct fuse_attr {
 	uint32_t	gid;
 	uint32_t	rdev;
 	uint32_t	blksize;
-	uint32_t	padding;
+	uint32_t	flags;
 };
 
 struct fuse_kstatfs {
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index 046a1b4a02..d8a3873246 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -60,6 +60,9 @@ static void convert_stat(const struct stat *stbuf, struct fuse_attr *attr)
 	attr->atimensec = ST_ATIM_NSEC(stbuf);
 	attr->mtimensec = ST_MTIM_NSEC(stbuf);
 	attr->ctimensec = ST_CTIM_NSEC(stbuf);
+
+	if (stbuf->st_size >= 1048576)
+		attr->flags |= FUSE_ATTR_DAX;
 }
 
 static void convert_attr(const struct fuse_setattr_in *attr, struct stat *stbuf)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH] virtiofsd: support per-file DAX
@ 2021-07-16  1:52             ` Jeffle Xu
  0 siblings, 0 replies; 22+ messages in thread
From: Jeffle Xu @ 2021-07-16  1:52 UTC (permalink / raw)
  To: vgoyal; +Cc: linux-fsdevel, virtualization, bo.liu, stefanha, miklos

An example implementation of supporting per-file DAX flag for virtiofsd,
where DAx is enabled for files larger than 1M size.

Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 contrib/virtiofsd/fuse_kernel.h   | 4 +++-
 contrib/virtiofsd/fuse_lowlevel.c | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
index d2b7ccf96b..9c476b7021 100644
--- a/contrib/virtiofsd/fuse_kernel.h
+++ b/contrib/virtiofsd/fuse_kernel.h
@@ -165,6 +165,8 @@
 /** The node ID of the root inode */
 #define FUSE_ROOT_ID 1
 
+#define FUSE_ATTR_DAX  (1 << 1)
+
 /* Make sure all structures are padded to 64bit boundary, so 32bit
    userspace works under 64bit kernels */
 
@@ -184,7 +186,7 @@ struct fuse_attr {
 	uint32_t	gid;
 	uint32_t	rdev;
 	uint32_t	blksize;
-	uint32_t	padding;
+	uint32_t	flags;
 };
 
 struct fuse_kstatfs {
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index 046a1b4a02..d8a3873246 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -60,6 +60,9 @@ static void convert_stat(const struct stat *stbuf, struct fuse_attr *attr)
 	attr->atimensec = ST_ATIM_NSEC(stbuf);
 	attr->mtimensec = ST_MTIM_NSEC(stbuf);
 	attr->ctimensec = ST_CTIM_NSEC(stbuf);
+
+	if (stbuf->st_size >= 1048576)
+		attr->flags |= FUSE_ATTR_DAX;
 }
 
 static void convert_attr(const struct fuse_setattr_in *attr, struct stat *stbuf)
-- 
2.27.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
  2021-07-16  1:32           ` Vivek Goyal
@ 2021-07-16  1:59             ` JeffleXu
  -1 siblings, 0 replies; 22+ messages in thread
From: JeffleXu @ 2021-07-16  1:59 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: Liu Bo, stefanha, miklos, linux-fsdevel, virtualization



On 7/16/21 9:32 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
>>
>>
>> On 7/16/21 8:51 AM, Vivek Goyal wrote:
>>> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
>>>> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
>>>>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
>>>>> this file.
>>>>>
>>>>> When the per-file DAX flag changes for an *opened* file, the state of
>>>>> the file won't be updated until this file is closed and reopened later.
>>>>>
>>>>> Currently it is not implemented yet to change per-file DAX flag inside
>>>>> guest kernel, e.g., by chattr(1).
>>>>
>>>> Thanks for the patch, it looks good to me.
>>>>
>>>> I think it's a good starting point, what I'd like to discuss here is
>>>> whether we're going to let chattr to toggle the dax flag.
>>>
>>> I have the same question. Why not take chattr approach as taken
>>> by ext4/xfs as well.
>>>
>>> Vivek
>>
>> Thanks.
>>
>> We can implement the chattr approach as ext4/xfs do, if we have this use
>> scenario. It's an RFC patch, and I want to collect more feedback as soon
>> as possible.
> 
> I guess chattr approach will allow client (as well as server) to control
> which files should be DAX. While this approach allows only server to
> specify which files should use DAX. Given currently we let client
> control whether to use dax or not (-o dax), it probably will make
> sense to use chattr based approach?

Yes, changing the per-file DAX flag from guest side, such as by chattr,
may be needed for completeness. I will include this in the next version.

> 
> I will look at the patches. Do you have a corresponding user space
> implementation somewhere so that I can test it?

Thanks. I have sent the corresponding patch in-reply-to your mail.

-- 
Thanks,
Jeffle

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [RFC PATCH 3/3] fuse: add per-file DAX flag
@ 2021-07-16  1:59             ` JeffleXu
  0 siblings, 0 replies; 22+ messages in thread
From: JeffleXu @ 2021-07-16  1:59 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-fsdevel, virtualization, Liu Bo, stefanha, miklos



On 7/16/21 9:32 AM, Vivek Goyal wrote:
> On Fri, Jul 16, 2021 at 09:18:34AM +0800, JeffleXu wrote:
>>
>>
>> On 7/16/21 8:51 AM, Vivek Goyal wrote:
>>> On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:
>>>> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
>>>>> Add one flag for fuse_attr.flags indicating if DAX shall be enabled for
>>>>> this file.
>>>>>
>>>>> When the per-file DAX flag changes for an *opened* file, the state of
>>>>> the file won't be updated until this file is closed and reopened later.
>>>>>
>>>>> Currently it is not implemented yet to change per-file DAX flag inside
>>>>> guest kernel, e.g., by chattr(1).
>>>>
>>>> Thanks for the patch, it looks good to me.
>>>>
>>>> I think it's a good starting point, what I'd like to discuss here is
>>>> whether we're going to let chattr to toggle the dax flag.
>>>
>>> I have the same question. Why not take chattr approach as taken
>>> by ext4/xfs as well.
>>>
>>> Vivek
>>
>> Thanks.
>>
>> We can implement the chattr approach as ext4/xfs do, if we have this use
>> scenario. It's an RFC patch, and I want to collect more feedback as soon
>> as possible.
> 
> I guess chattr approach will allow client (as well as server) to control
> which files should be DAX. While this approach allows only server to
> specify which files should use DAX. Given currently we let client
> control whether to use dax or not (-o dax), it probably will make
> sense to use chattr based approach?

Yes, changing the per-file DAX flag from guest side, such as by chattr,
may be needed for completeness. I will include this in the next version.

> 
> I will look at the patches. Do you have a corresponding user space
> implementation somewhere so that I can test it?

Thanks. I have sent the corresponding patch in-reply-to your mail.

-- 
Thanks,
Jeffle
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-07-16  1:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15  9:30 [RFC PATCH 0/3] virtiofs,fuse: support per-file DAX Jeffle Xu
2021-07-15  9:30 ` Jeffle Xu
2021-07-15  9:30 ` [RFC PATCH 1/3] fuse: add fuse_should_enable_dax() helper Jeffle Xu
2021-07-15  9:30   ` Jeffle Xu
2021-07-15  9:30 ` [RFC PATCH 2/3] fuse: Make DAX mount option a tri-state Jeffle Xu
2021-07-15  9:30   ` Jeffle Xu
2021-07-15  9:30 ` [RFC PATCH 3/3] fuse: add per-file DAX flag Jeffle Xu
2021-07-15  9:30   ` Jeffle Xu
2021-07-15 15:13   ` kernel test robot
2021-07-16  1:09     ` JeffleXu
2021-07-15 15:24   ` kernel test robot
2021-07-16  0:40   ` Liu Bo
2021-07-16  0:51     ` Vivek Goyal
2021-07-16  0:51       ` Vivek Goyal
2021-07-16  1:18       ` JeffleXu
2021-07-16  1:18         ` JeffleXu
2021-07-16  1:32         ` Vivek Goyal
2021-07-16  1:32           ` Vivek Goyal
2021-07-16  1:52           ` [PATCH] virtiofsd: support per-file DAX Jeffle Xu
2021-07-16  1:52             ` Jeffle Xu
2021-07-16  1:59           ` [RFC PATCH 3/3] fuse: add per-file DAX flag JeffleXu
2021-07-16  1:59             ` JeffleXu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.