Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Jan Kara <jack@suse.cz>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] fsnotify: Rearrange fast path to minimise overhead when there is no watcher
Date: Wed, 10 Jun 2020 16:24:02 +0300	[thread overview]
Message-ID: <CAOQ4uxhcFO4=e-s7uStbEkZU==8kraD1=owZGu4SWx_iR72gTA@mail.gmail.com> (raw)
In-Reply-To: <20200610125920.GM3127@techsingularity.net>

On Wed, Jun 10, 2020 at 3:59 PM Mel Gorman <mgorman@techsingularity.net> wrote:
>
> On Mon, Jun 08, 2020 at 11:39:26PM +0300, Amir Goldstein wrote:
> > > Let me add your optimizations on top of my branch with the needed
> > > adaptations and send you a branch for testing.
> >
> > https://github.com/amir73il/linux/commits/fsnotify_name-for-mel
> >
>
> Sorry for the delay getting back. The machine was busy with other tests
> and it took a while to reach this on the queue. Fairly good news
>
> hackbench-process-pipes
>                               5.7.0                  5.7.0                  5.7.0                  5.7.0
>                             vanilla      fastfsnotify-v1r1          amir-20200608           amir-for-mel
> Amean     1       0.4837 (   0.00%)      0.4630 *   4.27%*      0.4967 *  -2.69%*      0.4680 (   3.24%)
> Amean     3       1.5447 (   0.00%)      1.4557 (   5.76%)      1.6587 *  -7.38%*      1.4807 (   4.14%)
> Amean     5       2.6037 (   0.00%)      2.4363 (   6.43%)      2.6400 (  -1.40%)      2.4900 (   4.37%)
> Amean     7       3.5987 (   0.00%)      3.4757 (   3.42%)      3.9040 *  -8.48%*      3.5130 (   2.38%)
> Amean     12      5.8267 (   0.00%)      5.6983 (   2.20%)      6.2593 (  -7.43%)      5.6967 (   2.23%)
> Amean     18      8.4400 (   0.00%)      8.1327 (   3.64%)      8.9940 (  -6.56%)      7.7240 *   8.48%*
> Amean     24     11.0187 (   0.00%)     10.0290 *   8.98%*     11.7247 *  -6.41%*      9.5793 *  13.06%*
> Amean     30     13.1013 (   0.00%)     12.8510 (   1.91%)     14.0290 *  -7.08%*     12.1630 (   7.16%)
> Amean     32     13.9190 (   0.00%)     13.2410 (   4.87%)     14.7140 *  -5.71%*     13.2457 *   4.84%*
>
> First two sets of results are vanilla kernel and just my patch respectively
> to have two baselines. amir-20200608 is the first git branch you pointed
> me to and amir-for-mel is this latest branch. Comparing the optimisation
> and your series, we get
>
> hackbench-process-pipes
>                               5.7.0                  5.7.0
>                   fastfsnotify-v1r1           amir-for-mel
> Amean     1       0.4630 (   0.00%)      0.4680 (  -1.08%)
> Amean     3       1.4557 (   0.00%)      1.4807 (  -1.72%)
> Amean     5       2.4363 (   0.00%)      2.4900 (  -2.20%)
> Amean     7       3.4757 (   0.00%)      3.5130 (  -1.07%)
> Amean     12      5.6983 (   0.00%)      5.6967 (   0.03%)
> Amean     18      8.1327 (   0.00%)      7.7240 (   5.03%)
> Amean     24     10.0290 (   0.00%)      9.5793 (   4.48%)
> Amean     30     12.8510 (   0.00%)     12.1630 (   5.35%)
> Amean     32     13.2410 (   0.00%)     13.2457 (  -0.04%)
>
> As you can see, your patches with the optimisation layered on top is
> comparable to just the optimisation on its own. It's not universally
> better but it would not look like a regression when comparing releases.
> The differences are mostly within the noise as there is some variability
> involved for this workload so I would not worry too much about it (caveats
> are other machines may be different as well as other workloads).

Excellent!
Thanks for verifying.

TBH, this result is not surprising, because despite all the changes from
fastfsnotify-v1r1 to amir-for-mel, the code that is executed when there
are no watches should be quite similar. Without any unexpected compiler
optimizations that may differ between our versions, fsnotify hooks called
for directories should execute the exact same code.

fsnotify hooks called for non-directories (your workload) should execute
almost the same code. I spotted one additional test for access to
d_inode and one additional test of:
dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED
in the entry to __fsnotify_parent().

> A minor
> issue is that this is probably a bisection hazard. If a series is merged
> and LKP points the finger somewhere in the middle of your series then
> I suggest you ask them to test the optimisation commit ID to see if the
> regression goes away.
>

No worries. I wasn't planning on submitted the altered patch.
I just wanted to let you test the final result.
I will apply your change before my series and make sure to keep
optimizations while my changes are applied on top of that.

Thanks,
Amir.

  reply	other threads:[~2020-06-10 13:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-08 14:05 Mel Gorman
2020-06-08 15:03 ` Amir Goldstein
2020-06-08 16:06   ` Mel Gorman
2020-06-08 16:26     ` Amir Goldstein
2020-06-08 18:01       ` Mel Gorman
2020-06-08 18:12         ` Amir Goldstein
2020-06-08 20:39           ` Amir Goldstein
2020-06-10 12:59             ` Mel Gorman
2020-06-10 13:24               ` Amir Goldstein [this message]
2020-06-12  9:38                 ` Amir Goldstein
2020-06-12 12:42                   ` Mel Gorman
2020-06-08 15:19 ` Jan Kara
2020-06-08 16:50   ` Mel Gorman
2020-06-08 17:45     ` Amir Goldstein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxhcFO4=e-s7uStbEkZU==8kraD1=owZGu4SWx_iR72gTA@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@techsingularity.net \
    --subject='Re: [PATCH] fsnotify: Rearrange fast path to minimise overhead when there is no watcher' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).