On Wed, 2015-06-24 at 17:16 -0700, Dave Hansen wrote: > From: Dave Hansen <dave.han...@linux.intel.com> > > I have a _tiny_ microbenchmark that sits in a loop and writes > single bytes to a file. Writing one byte to a tmpfs file is > around 2x slower than reading one byte from a file, which is a > _bit_ more than I expecte. This is a dumb benchmark, but I think > it's hard to deny that write() is a hot path and we should avoid > unnecessary overhead there. > > I did a 'perf record' of 30-second samples of read and write. > The top item in a diffprofile is srcu_read_lock() from > fsnotify(). There are active inotify fd's from systemd, but > nothing is actually listening to the file or its part of > the filesystem. > > I *think* we can avoid taking the srcu_read_lock() for the > common case where there are no actual marks on the file. > This means that there will both be nothing to notify for > *and* implies that there is no need for clearing the ignore > mask. > > This patch gave a 13.8% speedup in writes/second on my test, > which is an improvement from the 10.8% that I saw with the > last version. > > Signed-off-by: Dave Hansen <dave.han...@linux.intel.com> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: Jan Kara <j...@suse.cz> > Cc: Al Viro <v...@zeniv.linux.org.uk> > Cc: Eric Paris <epa...@redhat.com> > Cc: John McCutchan <j...@johnmccutchan.com> > Cc: Robert Love <rl...@rlove.org> > Cc: Tim Chen <tim.c.c...@linux.intel.com> > Cc: Andi Kleen <a...@linux.intel.com> > Cc: linux-kernel@vger.kernel.org > --- > > b/fs/notify/fsnotify.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff -puN fs/notify/fsnotify.c~optimize-fsnotify fs/notify/fsnotify.c > --- a/fs/notify/fsnotify.c~optimize-fsnotify 2015-06-24 > 17:14:34.573109264 -0700 > +++ b/fs/notify/fsnotify.c 2015-06-24 17:14:34.576109399 -0700 > @@ -213,6 +213,16 @@ int fsnotify(struct inode *to_tell, __u3 > !(test_mask & to_tell->i_fsnotify_mask) && > !(mnt && test_mask & mnt->mnt_fsnotify_mask)) > return 0; > + /* > + * Optimization: srcu_read_lock() has a memory barrier which > can > + * be expensive. It protects walking the *_fsnotify_marks > lists. > + * However, if we do not walk the lists, we do not have to > do > + * SRCU because we have no references to any objects and do > not > + * need SRCU to keep them "alive". > + */ > + if (!to_tell->i_fsnotify_marks.first && > + (!mnt || !mnt->mnt_fsnotify_marks.first)) > + return 0;
two useless peeps from the old peanut gallery of long lost.... 1) should you actually move this check up before the IN_MODIFY check? This seems like it would be by far the most common case, and you'd save yourself a bunch of useless conditionals/bit operations. 2) do you want to use hlist_empty(&to_tell->i_fsnotify_marks) instead, for readability (and they are static inline, so compiled code is the same) It is fine as it is. Don't know how much you want to try to bikeshed... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
