smarter atime updates

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

#define UPDATE_ATIME(inode)                     \
do {                                            \
        if ((inode)->i_atime != CURRENT_TIME)   \
                update_atime (inode);           \

} while (0)

Quoted 7 lines · OGAWA Hirofumi

#define UPDATE_ATIME(inode)                     \
do {                                            \
        if ((inode)->i_atime != CURRENT_TIME)   \
                update_atime (inode);           \

} while (0)

yes, that'd be fine. The more conventional approach would be to blow away the strange UPPER CASE name and:

static inline void update_atime(struct inode *inode)

{
	if (inode->i_atime != CURRENT_TIME)
		__update_atime(inode);
}

But that would be a bigger patch, and I rather like shaving off three quarters of the sys_read() overhead with a two-liner ;)

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

fs/inode.c +2 -0

@@ -1187,6 +1187,8 @@ void __init inode_init(unsigned long mem
 
 void update_atime (struct inode *inode)
 {
+	if (inode->i_atime == CURRENT_TIME)
+		return;
 	if ( IS_NOATIME (inode) ) return;
 	if ( IS_NODIRATIME (inode) && S_ISDIR (inode->i_mode) ) return;
 	if ( IS_RDONLY (inode) ) return;

with this patch, the time to read a 10 meg file with 10 million read()s falls from 38 seconds (ext3), 39 seconds (reiserfs) and 11.6 seconds (ext2) down to 10.5 seconds.

-

Now are you sure this can't break anything ?

Quoted 20 lines

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

fs/inode.c +2 -0

@@ -1187,6 +1187,8 @@ void __init inode_init(unsigned long mem
 
 void update_atime (struct inode *inode)
 {
+	if (inode->i_atime == CURRENT_TIME)
+		return;
 	if ( IS_NOATIME (inode) ) return;
 	if ( IS_NODIRATIME (inode) && S_ISDIR (inode->i_mode) ) return;
 	if ( IS_RDONLY (inode) ) return;

with this patch, the time to read a 10 meg file with 10 million read()s falls from 38 seconds (ext3), 39 seconds (reiserfs) and 11.6 seconds (ext2) down to 10.5 seconds.

Now are you sure this can't break anything ?

Now are you sure this can't break anything ?

Quoted 20 lines · Andrew Morton

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

fs/inode.c +2 -0

@@ -1187,6 +1187,8 @@ void __init inode_init(unsigned long mem
 
 void update_atime (struct inode *inode)
 {
+	if (inode->i_atime == CURRENT_TIME)
+		return;
 	if ( IS_NOATIME (inode) ) return;
 	if ( IS_NODIRATIME (inode) && S_ISDIR (inode->i_mode) ) return;
 	if ( IS_RDONLY (inode) ) return;

with this patch, the time to read a 10 meg file with 10 million read()s falls from 38 seconds (ext3), 39 seconds (reiserfs) and 11.6 seconds (ext2) down to 10.5 seconds.

Quoted 2 lines

-

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Now are you sure this can't break anything ?

Quoted 16 lines

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

fs/inode.c +2 -0

@@ -1187,6 +1187,8 @@ void __init inode_init(unsigned long mem
 
 void update_atime (struct inode *inode)
 {
+	if (inode->i_atime == CURRENT_TIME)
+		return;
 	if ( IS_NOATIME (inode) ) return;
 	if ( IS_NODIRATIME (inode) && S_ISDIR (inode->i_mode) ) return;
 	if ( IS_RDONLY (inode) ) return;

Quoted 3 lines

with this patch, the time to read a 10 meg file with 10 million read()s falls from 38 seconds (ext3), 39 seconds (reiserfs) and 11.6 seconds (ext2) down to 10.5 seconds.

Well, just doing a code check of the update_atime() and UPDATE_ATIME() users, and they are all in readlink(), follow_link(), open_namei(), and various fs _readdir() codes. None of them (AFAICS) depend on the mark_inode_dirty() as a side-effect. This means it should be safe.

Quoted 5 lines · Andreas Dilger

Well, just doing a code check of the update_atime() and UPDATE_ATIME() users, and they are all in readlink(), follow_link(), open_namei(), and various fs _readdir() codes. None of them (AFAICS) depend on the mark_inode_dirty() as a side-effect. This means it should be safe.

More importantly, _if_ somebody depended on the side effects, they'd have been thwarted by the "noatime" mount option anyway, so any such bug would not be a new bug.

I've always thought filesystems should mount with noatime,nodiratime by default and only actually update atime if specifically mounted with "atime", as it's so rarely used. Out of all of the servers here, none actually use atime (every file system on _every_ server is mounted noatime,nodiratime). It's such a waste and just sounds fundamentally broken to issue a write because somebody read from a file.

...But there's probably some POSIX standard which would make such a change illegal. Blah blah...

(Not to say that atime isn't useful, but in most cases where it might be useful, it is so easily broken by backup processes, etc., that it really wants to be a different sort of mechanism.)

Hi,

Quoted 6 lines · Andrew Morton

mark_inode_dirty() is quite expensive for journalling filesystems, and we're calling it a lot more than we need to.

fs/inode.c void __init inode_init(unsigned long mem · line 1194

Quoted 8 lines

fs/inode.c +2 -0

@@ -1187,6 +1187,8 @@ void __init inode_init(unsigned long mem
 
 void update_atime (struct inode *inode)
 {
+	if (inode->i_atime == CURRENT_TIME)
+		return;
 	if ( IS_NOATIME (inode) ) return;
 	if ( IS_NODIRATIME (inode) && S_ISDIR (inode->i_mode) ) return;
 	if ( IS_RDONLY (inode) ) return;

in include/linux/fs.h:

#define UPDATE_ATIME(inode) \

do {						\
	if ((inode)->i_atime != CURRENT_TIME)	\
		update_atime (inode);		\
} while (0)

How about this macro? (add likely()?)