fs: introduce inode operation ->update_time

Btrfs has to make sure we have space to allocate new blocks in order to modify the inode, so updating time can fail. We've gotten around this by having our own file_update_time but this is kind of a pain, and Christoph has indicated he would like to make xfs do something different with atime updates. So introduce ->update_time, where we will deal with i_version an a/m/c time updates and indicate which changes need to be made. The normal version just does what it has always done, updates the time and marks the inode dirty, and then filesystems can choose to do something different. I've gone through all of the users of file_update_time and made them check for errors with the exception of the fault code since it's complicated and I wasn't quite sure what to do there, also Jan is going to be pushing the file time updates into page_mkwrite for those who have it so that should satisfy btrfs and make it not a big deal to check the file_update_time() return code in the generic fault path. Thanks, Signed-off-by: Josef Bacik <josef@redhat.com>

fs: introduce inode operation ->update_time
Btrfs has to make sure we have space to allocate new blocks in order to modify the inode, so updating time can fail. We've gotten around this by having our own file_update_time but this is kind of a pain, and Christoph has indicated he would like to make xfs do something different with atime updates. So introduce ->update_time, where we will deal with i_version an a/m/c time updates and indicate which changes need to be made. The normal version just does what it has always done, updates the time and marks the inode dirty, and then filesystems can choose to do something different. I've gone through all of the users of file_update_time and made them check for errors with the exception of the fault code since it's complicated and I wasn't quite sure what to do there, also Jan is going to be pushing the file time updates into page_mkwrite for those who have it so that should satisfy btrfs and make it not a big deal to check the file_update_time() return code in the generic fault path. Thanks, Signed-off-by: Josef Bacik <josef@redhat.com>
Josef Bacik · Al Viro
1 parent 033369d1af
Showing 12 changed files with 86 additions and 29 deletions Side-by-side Diff
Documentation/filesystems/Locking
Documentation/filesystems/vfs.txt
fs/fuse/file.c
fs/inode.c
fs/ncpfs/file.c
fs/ntfs/file.c
fs/pipe.c
fs/splice.c
fs/xfs/xfs_file.c
include/linux/fs.h
mm/filemap.c
mm/filemap_xip.c
@@ -62,6 +62,7 @@
 	int (*removexattr) (struct dentry *, const char *);
 	void (*truncate_range)(struct inode *, loff_t, loff_t);
 	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start, u64 len);
+	void (*update_time)(struct inode *, struct timespec *, int);
  
 locking rules:
 	all may block
@@ -89,6 +90,8 @@
 removexattr:	yes
 truncate_range:	yes
 fiemap:		no
+update_time:	no
+
 	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
@@ -364,6 +364,7 @@
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
 	int (*removexattr) (struct dentry *, const char *);
 	void (*truncate_range)(struct inode *, loff_t, loff_t);
+	void (*update_time)(struct inode *, struct timespec *, int);
 };
  
 Again, all methods are called without any locks being held, unless
@@ -475,6 +476,9 @@
   truncate_range: a method provided by the underlying filesystem to truncate a
   	range of blocks , i.e. punch a hole somewhere in a file.
  
+  update_time: called by the VFS to update a specific time or the i_version of
+  	an inode.  If this is not defined the VFS will update the inode itself
+  	and call mark_inode_dirty_sync.
  
 The Address Space Object
 ========================
@@ -962,7 +962,9 @@
 	if (err)
 		goto out;
  
-	file_update_time(file);
+	err = file_update_time(file);
+	if (err)
+		goto out;
  
 	if (file->f_flags & O_DIRECT) {
 		written = generic_file_direct_write(iocb, iov, &nr_segs,
@@ -1487,6 +1487,27 @@
 	return 0;
 }
  
+/*
+ * This does the actual work of updating an inodes time or version.  Must have
+ * had called mnt_want_write() before calling this.
+ */
+static int update_time(struct inode *inode, struct timespec *time, int flags)
+{
+	if (inode->i_op->update_time)
+		return inode->i_op->update_time(inode, time, flags);
+
+	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;
+	mark_inode_dirty_sync(inode);
+	return 0;
+}
+
 /**
  *	touch_atime	-	update the access time
  *	@path: the &struct path to update
@@ -1524,8 +1545,14 @@
 	if (mnt_want_write(mnt))
 		return;
  
-	inode->i_atime = now;
-	mark_inode_dirty_sync(inode);
+	/*
+	 * File systems can error out when updating inodes if they need to
+	 * allocate new space to modify an inode (such is the case for
+	 * Btrfs), but since we touch atime while walking down the path we
+	 * really don't care if we failed to update the atime of the file,
+	 * so just ignore the return value.
+	 */
+	update_time(inode, &now, S_ATIME);
 	mnt_drop_write(mnt);
 }
 EXPORT_SYMBOL(touch_atime);
  
  
  
@@ -1604,18 +1631,20 @@
  *	usage in the file write path of filesystems, and filesystems may
  *	choose to explicitly ignore update via this function with the
  *	S_NOCMTIME inode flag, e.g. for network filesystem where these
- *	timestamps are handled by the server.
+ *	timestamps are handled by the server.  This can return an error for
+ *	file systems who need to allocate space in order to update an inode.
  */
  
-void file_update_time(struct file *file)
+int file_update_time(struct file *file)
 {
 	struct inode *inode = file->f_path.dentry->d_inode;
 	struct timespec now;
-	enum { S_MTIME = 1, S_CTIME = 2, S_VERSION = 4 } sync_it = 0;
+	int sync_it = 0;
+	int ret;
  
 	/* First try to exhaust all avenues to not sync */
 	if (IS_NOCMTIME(inode))
-		return;
+		return 0;
  
 	now = current_fs_time(inode->i_sb);
 	if (!timespec_equal(&inode->i_mtime, &now))
  
  
  
@@ -1628,21 +1657,16 @@
 		sync_it |= S_VERSION;
  
 	if (!sync_it)
-		return;
+		return 0;
  
 	/* Finally allowed to write? Takes lock. */
 	if (mnt_want_write_file(file))
-		return;
+		return 0;
  
-	/* Only change inode inside the lock region */
-	if (sync_it & S_VERSION)
-		inode_inc_iversion(inode);
-	if (sync_it & S_CTIME)
-		inode->i_ctime = now;
-	if (sync_it & S_MTIME)
-		inode->i_mtime = now;
-	mark_inode_dirty_sync(inode);
+	ret = update_time(inode, &now, sync_it);
 	mnt_drop_write_file(file);
+
+	return ret;
 }
 EXPORT_SYMBOL(file_update_time);
  
@@ -221,6 +221,10 @@
  
 	already_written = 0;
  
+	errno = file_update_time(file);
+	if (errno)
+		goto outrel;
+
 	bouncebuffer = vmalloc(bufsize);
 	if (!bouncebuffer) {
 		errno = -EIO;	/* -ENOMEM */
@@ -251,8 +255,6 @@
 		}
 	}
 	vfree(bouncebuffer);
-
-	file_update_time(file);
  
 	*ppos = pos;
  
@@ -2096,7 +2096,9 @@
 	err = file_remove_suid(file);
 	if (err)
 		goto out;
-	file_update_time(file);
+	err = file_update_time(file);
+	if (err)
+		goto out;
 	written = ntfs_file_buffered_write(iocb, iov, nr_segs, pos, ppos,
 			count);
 out:
@@ -654,8 +654,11 @@
 		wake_up_interruptible_sync_poll(&pipe->wait, POLLIN | POLLRDNORM);
 		kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
 	}
-	if (ret > 0)
-		file_update_time(filp);
+	if (ret > 0) {
+		int err = file_update_time(filp);
+		if (err)
+			ret = err;
+	}
 	return ret;
 }
  
@@ -1003,8 +1003,10 @@
 		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
 		ret = file_remove_suid(out);
 		if (!ret) {
-			file_update_time(out);
-			ret = splice_from_pipe_feed(pipe, &sd, pipe_to_file);
+			ret = file_update_time(out);
+			if (!ret)
+				ret = splice_from_pipe_feed(pipe, &sd,
+							    pipe_to_file);
 		}
 		mutex_unlock(&inode->i_mutex);
 	} while (ret > 0);
@@ -586,8 +586,11 @@
 	 * lock above.  Eventually we should look into a way to avoid
 	 * the pointless lock roundtrip.
 	 */
-	if (likely(!(file->f_mode & FMODE_NOCMTIME)))
-		file_update_time(file);
+	if (likely(!(file->f_mode & FMODE_NOCMTIME))) {
+		error = file_update_time(file);
+		if (error)
+			return error;
+	}
  
 	/*
 	 * If we're writing the file then make sure to clear the setuid and
@@ -1684,6 +1684,7 @@
 	void (*truncate_range)(struct inode *, loff_t, loff_t);
 	int (*fiemap)(struct inode *, struct fiemap_extent_info *, u64 start,
 		      u64 len);
+	int (*update_time)(struct inode *, struct timespec *, int);
 } ____cacheline_aligned;
  
 struct seq_file;
@@ -1843,6 +1844,13 @@
        spin_unlock(&inode->i_lock);
 }
  
+enum file_time_flags {
+	S_ATIME = 1,
+	S_MTIME = 2,
+	S_CTIME = 4,
+	S_VERSION = 8,
+};
+
 extern void touch_atime(struct path *);
 static inline void file_accessed(struct file *file)
 {
@@ -2579,7 +2587,7 @@
 extern int inode_newsize_ok(const struct inode *, loff_t offset);
 extern void setattr_copy(struct inode *inode, const struct iattr *attr);
  
-extern void file_update_time(struct file *file);
+extern int file_update_time(struct file *file);
  
 extern int generic_show_options(struct seq_file *m, struct dentry *root);
 extern void save_mount_options(struct super_block *sb, char *options);
@@ -2463,7 +2463,9 @@
 	if (err)
 		goto out;
  
-	file_update_time(file);
+	err = file_update_time(file);
+	if (err)
+		goto out;
  
 	/* coalesce the iovecs and go direct-to-BIO for O_DIRECT */
 	if (unlikely(file->f_flags & O_DIRECT)) {
@@ -426,7 +426,9 @@
 	if (ret)
 		goto out_backing;
  
-	file_update_time(filp);
+	ret = file_update_time(filp);
+	if (ret)
+		goto out_backing;
  
 	ret = __xip_file_write (filp, buf, count, pos, ppos);
...	...	@@ -62,6 +62,7 @@
62	62	int (removexattr) (struct dentry , const char *);
63	63	void (truncate_range)(struct inode , loff_t, loff_t);
64	64	int (fiemap)(struct inode , struct fiemap_extent_info *, u64 start, u64 len);
	65	+ void (update_time)(struct inode , struct timespec *, int);
65	66
66	67	locking rules:
67	68	all may block
...	...	@@ -89,6 +90,8 @@
89	90	removexattr: yes
90	91	truncate_range: yes
91	92	fiemap: no
	93	+update_time: no
	94	+
92	95	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_mutex on
93	96	victim.
94	97	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
...	...	@@ -364,6 +364,7 @@
364	364	ssize_t (listxattr) (struct dentry , char *, size_t);
365	365	int (removexattr) (struct dentry , const char *);
366	366	void (truncate_range)(struct inode , loff_t, loff_t);
	367	+ void (update_time)(struct inode , struct timespec *, int);
367	368	};
368	369
369	370	Again, all methods are called without any locks being held, unless
...	...	@@ -475,6 +476,9 @@
475	476	truncate_range: a method provided by the underlying filesystem to truncate a
476	477	range of blocks , i.e. punch a hole somewhere in a file.
477	478
	479	+ update_time: called by the VFS to update a specific time or the i_version of
	480	+ an inode. If this is not defined the VFS will update the inode itself
	481	+ and call mark_inode_dirty_sync.
478	482
479	483	The Address Space Object
480	484	========================
...	...	@@ -962,7 +962,9 @@
962	962	if (err)
963	963	goto out;
964	964
965		- file_update_time(file);
	965	+ err = file_update_time(file);
	966	+ if (err)
	967	+ goto out;
966	968
967	969	if (file->f_flags & O_DIRECT) {
968	970	written = generic_file_direct_write(iocb, iov, &nr_segs,
...	...	@@ -1487,6 +1487,27 @@
1487	1487	return 0;
1488	1488	}
1489	1489
	1490	+/*
	1491	+ * This does the actual work of updating an inodes time or version. Must have
	1492	+ * had called mnt_want_write() before calling this.
	1493	+ */
	1494	+static int update_time(struct inode inode, struct timespec time, int flags)
	1495	+{
	1496	+ if (inode->i_op->update_time)
	1497	+ return inode->i_op->update_time(inode, time, flags);
	1498	+
	1499	+ if (flags & S_ATIME)
	1500	+ inode->i_atime = *time;
	1501	+ if (flags & S_VERSION)
	1502	+ inode_inc_iversion(inode);
	1503	+ if (flags & S_CTIME)
	1504	+ inode->i_ctime = *time;
	1505	+ if (flags & S_MTIME)
	1506	+ inode->i_mtime = *time;
	1507	+ mark_inode_dirty_sync(inode);
	1508	+ return 0;
	1509	+}
	1510	+
1490	1511	/**
1491	1512	* touch_atime - update the access time
1492	1513	* @path: the &struct path to update
...	...	@@ -1524,8 +1545,14 @@
1524	1545	if (mnt_want_write(mnt))
1525	1546	return;
1526	1547
1527		- inode->i_atime = now;
1528		- mark_inode_dirty_sync(inode);
	1548	+ /*
	1549	+ * File systems can error out when updating inodes if they need to
	1550	+ * allocate new space to modify an inode (such is the case for
	1551	+ * Btrfs), but since we touch atime while walking down the path we
	1552	+ * really don't care if we failed to update the atime of the file,
	1553	+ * so just ignore the return value.
	1554	+ */
	1555	+ update_time(inode, &now, S_ATIME);
1529	1556	mnt_drop_write(mnt);
1530	1557	}
1531	1558	EXPORT_SYMBOL(touch_atime);
1532	1559
1533	1560
1534	1561
...	...	@@ -1604,18 +1631,20 @@
1604	1631	* usage in the file write path of filesystems, and filesystems may
1605	1632	* choose to explicitly ignore update via this function with the
1606	1633	* S_NOCMTIME inode flag, e.g. for network filesystem where these
1607		- * timestamps are handled by the server.
	1634	+ * timestamps are handled by the server. This can return an error for
	1635	+ * file systems who need to allocate space in order to update an inode.
1608	1636	*/
1609	1637
1610		-void file_update_time(struct file *file)
	1638	+int file_update_time(struct file *file)
1611	1639	{
1612	1640	struct inode *inode = file->f_path.dentry->d_inode;
1613	1641	struct timespec now;
1614		- enum { S_MTIME = 1, S_CTIME = 2, S_VERSION = 4 } sync_it = 0;
	1642	+ int sync_it = 0;
	1643	+ int ret;
1615	1644
1616	1645	/* First try to exhaust all avenues to not sync */
1617	1646	if (IS_NOCMTIME(inode))
1618		- return;
	1647	+ return 0;
1619	1648
1620	1649	now = current_fs_time(inode->i_sb);
1621	1650	if (!timespec_equal(&inode->i_mtime, &now))
1622	1651
1623	1652
1624	1653
...	...	@@ -1628,21 +1657,16 @@
1628	1657	sync_it \|= S_VERSION;
1629	1658
1630	1659	if (!sync_it)
1631		- return;
	1660	+ return 0;
1632	1661
1633	1662	/* Finally allowed to write? Takes lock. */
1634	1663	if (mnt_want_write_file(file))
1635		- return;
	1664	+ return 0;
1636	1665
1637		- /* Only change inode inside the lock region */
1638		- if (sync_it & S_VERSION)
1639		- inode_inc_iversion(inode);
1640		- if (sync_it & S_CTIME)
1641		- inode->i_ctime = now;
1642		- if (sync_it & S_MTIME)
1643		- inode->i_mtime = now;
1644		- mark_inode_dirty_sync(inode);
	1666	+ ret = update_time(inode, &now, sync_it);
1645	1667	mnt_drop_write_file(file);
	1668	+
	1669	+ return ret;
1646	1670	}
1647	1671	EXPORT_SYMBOL(file_update_time);
1648	1672
...	...	@@ -221,6 +221,10 @@
221	221
222	222	already_written = 0;
223	223
	224	+ errno = file_update_time(file);
	225	+ if (errno)
	226	+ goto outrel;
	227	+
224	228	bouncebuffer = vmalloc(bufsize);
225	229	if (!bouncebuffer) {
226	230	errno = -EIO; /* -ENOMEM */
...	...	@@ -251,8 +255,6 @@
251	255	}
252	256	}
253	257	vfree(bouncebuffer);
254		-
255		- file_update_time(file);
256	258
257	259	*ppos = pos;
258	260
...	...	@@ -2096,7 +2096,9 @@
2096	2096	err = file_remove_suid(file);
2097	2097	if (err)
2098	2098	goto out;
2099		- file_update_time(file);
	2099	+ err = file_update_time(file);
	2100	+ if (err)
	2101	+ goto out;
2100	2102	written = ntfs_file_buffered_write(iocb, iov, nr_segs, pos, ppos,
2101	2103	count);
2102	2104	out:
...	...	@@ -654,8 +654,11 @@
654	654	wake_up_interruptible_sync_poll(&pipe->wait, POLLIN \| POLLRDNORM);
655	655	kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
656	656	}
657		- if (ret > 0)
658		- file_update_time(filp);
	657	+ if (ret > 0) {
	658	+ int err = file_update_time(filp);
	659	+ if (err)
	660	+ ret = err;
	661	+ }
659	662	return ret;
660	663	}
661	664
...	...	@@ -1003,8 +1003,10 @@
1003	1003	mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
1004	1004	ret = file_remove_suid(out);
1005	1005	if (!ret) {
1006		- file_update_time(out);
1007		- ret = splice_from_pipe_feed(pipe, &sd, pipe_to_file);
	1006	+ ret = file_update_time(out);
	1007	+ if (!ret)
	1008	+ ret = splice_from_pipe_feed(pipe, &sd,
	1009	+ pipe_to_file);
1008	1010	}
1009	1011	mutex_unlock(&inode->i_mutex);
1010	1012	} while (ret > 0);
...	...	@@ -586,8 +586,11 @@
586	586	* lock above. Eventually we should look into a way to avoid
587	587	* the pointless lock roundtrip.
588	588	*/
589		- if (likely(!(file->f_mode & FMODE_NOCMTIME)))
590		- file_update_time(file);
	589	+ if (likely(!(file->f_mode & FMODE_NOCMTIME))) {
	590	+ error = file_update_time(file);
	591	+ if (error)
	592	+ return error;
	593	+ }
591	594
592	595	/*
593	596	* If we're writing the file then make sure to clear the setuid and
...	...	@@ -1684,6 +1684,7 @@
1684	1684	void (truncate_range)(struct inode , loff_t, loff_t);
1685	1685	int (fiemap)(struct inode , struct fiemap_extent_info *, u64 start,
1686	1686	u64 len);
	1687	+ int (update_time)(struct inode , struct timespec *, int);
1687	1688	} ____cacheline_aligned;
1688	1689
1689	1690	struct seq_file;
...	...	@@ -1843,6 +1844,13 @@
1843	1844	spin_unlock(&inode->i_lock);
1844	1845	}
1845	1846
	1847	+enum file_time_flags {
	1848	+ S_ATIME = 1,
	1849	+ S_MTIME = 2,
	1850	+ S_CTIME = 4,
	1851	+ S_VERSION = 8,
	1852	+};
	1853	+
1846	1854	extern void touch_atime(struct path *);
1847	1855	static inline void file_accessed(struct file *file)
1848	1856	{
...	...	@@ -2579,7 +2587,7 @@
2579	2587	extern int inode_newsize_ok(const struct inode *, loff_t offset);
2580	2588	extern void setattr_copy(struct inode inode, const struct iattr attr);
2581	2589
2582		-extern void file_update_time(struct file *file);
	2590	+extern int file_update_time(struct file *file);
2583	2591
2584	2592	extern int generic_show_options(struct seq_file m, struct dentry root);
2585	2593	extern void save_mount_options(struct super_block sb, char options);