From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751546AbbFYQ2f (ORCPT ); Thu, 25 Jun 2015 12:28:35 -0400 Received: from www.sr71.net ([198.145.64.142]:51001 "EHLO blackbird.sr71.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751239AbbFYQ21 (ORCPT ); Thu, 25 Jun 2015 12:28:27 -0400 Message-ID: <558C2C29.7040106@sr71.net> Date: Thu, 25 Jun 2015 09:28:25 -0700 From: Dave Hansen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Eric Paris CC: dave.hansen@linux.intel.com, akpm@linux-foundation.org, jack@suse.cz, viro@zeniv.linux.org.uk, john@johnmccutchan.com, rlove@rlove.org, tim.c.chen@linux.intel.com, ak@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [RFCv2][PATCH 1/7] fs: optimize inotify/fsnotify code for unwatched files References: <20150625001605.72553909@viggo.jf.intel.com> <1435193823.19444.36.camel@redhat.com> In-Reply-To: <1435193823.19444.36.camel@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/24/2015 05:57 PM, Eric Paris wrote: > On Wed, 2015-06-24 at 17:16 -0700, Dave Hansen wrote: >> + 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. Doing that actually makes fsnotify() 82 bytes smaller for me. I think you're also right that the new check is a much more general condition than the existing one. I'll move it up when I post again. > 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) Yeah I guess that makes sense. The only thing that the current code is nice for is that it makes it obvious that the .first checks match with the srcu_dereference()s done below.