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: Fri, 12 Jun 2020 12:38:15 +0300	[thread overview]
Message-ID: <CAOQ4uxjTFCJa1y2Uq8NztXxkPRmvDvtUUt22QMwPkdd=eJdkyw@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxhcFO4=e-s7uStbEkZU==8kraD1=owZGu4SWx_iR72gTA@mail.gmail.com>

On Wed, Jun 10, 2020 at 4:24 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> 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.
>

FYI, just posted your patch with a minor style change at the bottom
of my prep patch series.

Thanks,
Amir.

  reply	other threads:[~2020-06-12  9:38 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
2020-06-12  9:38                 ` Amir Goldstein [this message]
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='CAOQ4uxjTFCJa1y2Uq8NztXxkPRmvDvtUUt22QMwPkdd=eJdkyw@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).