[PATCH v5] ubifs: introduce UBIFS_ATIME_SUPPORT to ubifs

[PATCH v5] ubifs: introduce UBIFS_ATIME_SUPPORT to ubifs

[Dongsheng Yang]

To make ubifs support atime flexily, this commit introduces
a Kconfig option named as UBIFS_ATIME_SUPPORT.

With UBIFS_ATIME_SUPPORT=n:
	ubifs keeps the full compatibility to no_atime from
the start of ubifs.
=================UBIFS_ATIME_SUPPORT=n=======================
-o - no atime
-o atime - no atime
-o noatime - no atime
-o relatime - no atime
-o strictatime - no atime
-o lazyatime - no atime

With UBIFS_ATIME_SUPPORT=y:
	ubifs supports the atime same with other main stream
file systems.
=================UBIFS_ATIME_SUPPORT=y=======================
-o - default behavior (relatime currently)
-o atime - atime support
-o noatime - no atime support
-o relatime - relative atime support
-o strictatime - strict atime support
-o lazyatime - lazy atime support

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
	-v2:
		implement update_time for ubifs
	-v3:
		Oops, forgot to assign update_time for
		ubifs_dir_inode_operations.
	-v4:
		drop the patch to change 'Y' to 'N' in
		Kconfig
	-v5:
		simplify the warning in atime supporting
 fs/ubifs/Kconfig | 11 +++++++++++
 fs/ubifs/dir.c   |  3 +++
 fs/ubifs/file.c  | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ubifs/super.c | 12 ++++++++++--
 fs/ubifs/ubifs.h |  1 +
 5 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/Kconfig b/fs/ubifs/Kconfig
index ba66d50..2eaae3e 100644
--- a/fs/ubifs/Kconfig
+++ b/fs/ubifs/Kconfig
@@ -35,3 +35,14 @@ config UBIFS_FS_ZLIB
 	default y
 	help
 	  Zlib compresses better than LZO but it is slower. Say 'Y' if unsure.
+
+config UBIFS_ATIME_SUPPORT
+	bool "Access time support" if UBIFS_FS
+	depends on UBIFS_FS
+	default n
+	help
+	  This option allows ubifs to support atime. -o strictatime is harmful to
+	  your flash, we don't suggest it. But relatime and lazytime are much
+	  better.
+
+	  If unsure, say 'N'
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 27060fc..8d93427 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1187,6 +1187,9 @@ const struct inode_operations ubifs_dir_inode_operations = {
 	.getxattr    = ubifs_getxattr,
 	.listxattr   = ubifs_listxattr,
 	.removexattr = ubifs_removexattr,
+#ifdef CONFIG_UBIFS_ATIME_SUPPORT
+	.update_time = ubifs_update_time,
+#endif
 };
 
 const struct file_operations ubifs_dir_operations = {
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 35efc10..554d7b9 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1364,6 +1364,47 @@ static inline int mctime_update_needed(const struct inode *inode,
 }
 
 /**
+ * ubifs_update_time - update time of inode.
+ * @inode: inode to update
+ *
+ * This function updates time of the inode.
+ */
+int ubifs_update_time(struct inode *inode, struct timespec *time,
+			     int flags)
+{
+	struct ubifs_inode *ui = ubifs_inode(inode);
+	struct ubifs_info *c = inode->i_sb->s_fs_info;
+	struct ubifs_budget_req req = { .dirtied_ino = 1,
+			.dirtied_ino_d = ALIGN(ui->data_len, 8) };
+	int iflags = I_DIRTY_TIME;
+	int err, release;
+
+	err = ubifs_budget_space(c, &req);
+	if (err)
+		return err;
+
+	mutex_lock(&ui->ui_mutex);
+	if (flags & S_ATIME)
+		inode->i_atime = *time;
+	if (flags & S_VERSION)
+		inode_inc_iversion(inode);
+	if (flags & S_CTIME)
+		inode->i_ctime = *time;
+	if (flags & S_MTIME)
+		inode->i_mtime = *time;
+
+	if (!(inode->i_sb->s_flags & MS_LAZYTIME) || (flags & S_VERSION))
+		iflags |= I_DIRTY_SYNC;
+
+	release = ui->dirty;
+	__mark_inode_dirty(inode, iflags);
+	mutex_unlock(&ui->ui_mutex);
+	if (release)
+		ubifs_release_budget(c, &req);
+	return 0;
+}
+
+/**
  * update_ctime - update mtime and ctime of an inode.
  * @inode: inode to update
  *
@@ -1546,6 +1587,9 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma)
 	if (err)
 		return err;
 	vma->vm_ops = &ubifs_file_vm_ops;
+#ifdef CONFIG_UBIFS_ATIME_SUPPORT
+	file_accessed(file);
+#endif
 	return 0;
 }
 
@@ -1566,6 +1610,9 @@ const struct inode_operations ubifs_file_inode_operations = {
 	.getxattr    = ubifs_getxattr,
 	.listxattr   = ubifs_listxattr,
 	.removexattr = ubifs_removexattr,
+#ifdef CONFIG_UBIFS_ATIME_SUPPORT
+	.update_time = ubifs_update_time,
+#endif
 };
 
 const struct inode_operations ubifs_symlink_inode_operations = {
@@ -1577,6 +1624,9 @@ const struct inode_operations ubifs_symlink_inode_operations = {
 	.getxattr    = ubifs_getxattr,
 	.listxattr   = ubifs_listxattr,
 	.removexattr = ubifs_removexattr,
+#ifdef CONFIG_UBIFS_ATIME_SUPPORT
+	.update_time = ubifs_update_time,
+#endif
 };
 
 const struct file_operations ubifs_file_operations = {
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 75e6f04..ab9e9b3 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -128,7 +128,10 @@ struct inode *ubifs_iget(struct super_block *sb, unsigned long inum)
 	if (err)
 		goto out_ino;
 
-	inode->i_flags |= (S_NOCMTIME | S_NOATIME);
+	inode->i_flags |= S_NOCMTIME;
+#ifndef CONFIG_UBIFS_ATIME_SUPPORT
+	inode->i_flags |= S_NOATIME;
+#endif
 	set_nlink(inode, le32_to_cpu(ino->nlink));
 	i_uid_write(inode, le32_to_cpu(ino->uid));
 	i_gid_write(inode, le32_to_cpu(ino->gid));
@@ -2138,7 +2141,12 @@ static struct dentry *ubifs_mount(struct file_system_type *fs_type, int flags,
 		if (err)
 			goto out_deact;
 		/* We do not support atime */
-		sb->s_flags |= MS_ACTIVE | MS_NOATIME;
+		sb->s_flags |= MS_ACTIVE;
+#ifndef CONFIG_UBIFS_ATIME_SUPPORT
+		sb->s_flags |= MS_NOATIME;
+#else
+		ubifs_warn(c, "full atime support is enabled, which may wear out your flash faster");
+#endif
 	}
 
 	/* 'fill_super()' opens ubi again so we must close it here */
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index de75902..216ba87 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -1746,6 +1746,7 @@ int ubifs_calc_dark(const struct ubifs_info *c, int spc);
 /* file.c */
 int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync);
 int ubifs_setattr(struct dentry *dentry, struct iattr *attr);
+int ubifs_update_time(struct inode *inode, struct timespec *time, int flags);
 
 /* dir.c */
 struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
-- 
1.8.4.2

Re: [PATCH v5] ubifs: introduce UBIFS_ATIME_SUPPORT to ubifs

[Artem Bityutskiy]

On Tue, 2015-08-18 at 12:40 +0800, Dongsheng Yang wrote:
> To make ubifs support atime flexily, this commit introduces
> a Kconfig option named as UBIFS_ATIME_SUPPORT.
> 
> With UBIFS_ATIME_SUPPORT=n:
> 	ubifs keeps the full compatibility to no_atime from
> the start of ubifs.
> =================UBIFS_ATIME_SUPPORT=n=======================
> -o - no atime
> -o atime - no atime
> -o noatime - no atime
> -o relatime - no atime
> -o strictatime - no atime
> -o lazyatime - no atime

And please, do include fsdevel when sending this patch.

Artem.

Re: [PATCH v5] ubifs: introduce UBIFS_ATIME_SUPPORT to ubifs

[Dongsheng Yang]

On 08/18/2015 04:43 PM, Artem Bityutskiy wrote:
> Please, take a look at the mkwrite path, most of the file-systems update
> atime there too,
> I think your patch may need to do something about atime in
> 'ubifs_vm_page_mkwrite()'.

Hi Atem,
	most of the file-systems are updating the ctime and mtime
in mkwrite path by calling file_update_time().It's a writing path
so we don't update atime here.
>
> On Tue, Aug 18, 2015 at 7:40 AM, Dongsheng Yang
> <yangds.fnst@cn.fujitsu.com <mailto:yangds.fnst@cn.fujitsu.com>> wrote:
>
>     To make ubifs support atime flexily, this commit introduces
>     +config UBIFS_ATIME_SUPPORT
>     +       bool "Access time support" if UBIFS_FS
>     +       depends on UBIFS_FS
>     +       default n
>     +       help
>     +         This option allows ubifs to support atime. -o strictatime
>     is harmful to
>     +         your flash, we don't suggest it. But relatime and lazytime
>     are much
>     +         better.
>
>
> How about this.
>
> Originally UBIFS did not support atime, because it looked like a bad
> idea due
> increased flash wear. This option adds atime support and it is disabled
> by default
> to preserve the old behavior. If you enable this option, UBIFS starts
> updating atime,
> which means that file-system read operations will cause writes (inode atime
> updates). This may affect file-system performance and increase flash
> device wear,
> so be careful. How often atime is updated depends on the selected strategy:
> strictatime is the "heavy", relatime is "lighter", etc.

Much better :)
>
>
>     +int ubifs_update_time(struct inode *inode, struct timespec *time,
>     +                            int flags)
>     +{
>     +       struct ubifs_inode *ui = ubifs_inode(inode);
>     +       struct ubifs_info *c = inode->i_sb->s_fs_info;
>     +       struct ubifs_budget_req req = { .dirtied_ino = 1,
>     +                       .dirtied_ino_d = ALIGN(ui->data_len, 8) };
>     +       int iflags = I_DIRTY_TIME;
>     +       int err, release;
>     +
>     +       err = ubifs_budget_space(c, &req);
>     +       if (err)
>     +               return err;
>     +
>     +       mutex_lock(&ui->ui_mutex);
>
>     +       if (flags & S_ATIME)
>     +               inode->i_atime = *time;
>     +       if (flags & S_VERSION)
>     +               inode_inc_iversion(inode);
>     +       if (flags & S_CTIME)
>     +               inode->i_ctime = *time;
>     +       if (flags & S_MTIME)
>     +               inode->i_mtime = *time;
>     +
>     +       if (!(inode->i_sb->s_flags & MS_LAZYTIME) || (flags &
>     S_VERSION))
>     +               iflags |= I_DIRTY_SYNC;
>
>
> The lazytime part looks OK, but I am a bit concerned about the S_VERSION
> part.
> IIRC, the inode version stuff is needed for NFS, which UBIFS does not
> support
> anyway. I do not see that we do anything with inode version in UBIFS, so
> it looks
> like UBIFS just does not support this.
>
> I do not think we should add code we do not really need or understand.
> Could you
> please take a closer look to the S_VERSION stuff and either remove it
> from this
> patch or make sure UBIFS needs this piece of code, which I doubt.

Thanx, I looked some more about it.

Hmmm, yes, agree that we should not introduce some code here
to make it hard for understanding, although that would never
cause some problem. I will remove it.

Maybe we can reintroduce it when we plan to add nfs supporting
in ubifs.:)
>
>
>     -               sb->s_flags |= MS_ACTIVE | MS_NOATIME;
>     +               sb->s_flags |= MS_ACTIVE;
>     +#ifndef CONFIG_UBIFS_ATIME_SUPPORT
>     +               sb->s_flags |= MS_NOATIME;
>     +#else
>     +               ubifs_warn(c, "full atime support is enabled, which
>     may wear out your flash faster");
>     +#endif
>
>
> I am not sure we need to print this warning. If I know what I am doing,
> and want atime,
> why whould I see the warning?
>
> We made the default to be the old behavior, whoudn't it be enough to
> just print a
> message (ubifs_info()) about enabled atime, without the "which may wear
> out your flash
> faster" part, what do you think?

Hmmm "default behavior to disable" makes sense to me. Yes, if someone
  enable it by intention, we need not warning it out to him. Okey,
Just a ubifs_info sounds good to me.

Yang
>
> Thanks!
>
> --
> Best Regards,
> Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH v5] ubifs: introduce UBIFS_ATIME_SUPPORT to ubifs

[Dongsheng Yang]

On 08/18/2015 04:44 PM, Artem Bityutskiy wrote:
> On Tue, 2015-08-18 at 12:40 +0800, Dongsheng Yang wrote:
>> To make ubifs support atime flexily, this commit introduces
>> a Kconfig option named as UBIFS_ATIME_SUPPORT.
>>
>> With UBIFS_ATIME_SUPPORT=n:
>> 	ubifs keeps the full compatibility to no_atime from
>> the start of ubifs.
>> =================UBIFS_ATIME_SUPPORT=n=======================
>> -o - no atime
>> -o atime - no atime
>> -o noatime - no atime
>> -o relatime - no atime
>> -o strictatime - no atime
>> -o lazyatime - no atime
>
> And please, do include fsdevel when sending this patch.

Sorry again. will cc it in next verison. :(
>
> Artem.
> .
>