lkml.org 
[lkml]   [2018]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 4.4 50/92] mm: filemap: avoid unnecessary calls to lock_page when waiting for IO to complete during a read
On Thu, May 24, 2018 at 12:06 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, May 24, 2018 at 10:27:59AM -0700, Hugh Dickins wrote:
> > On Thu, May 24, 2018 at 4:28 AM Greg KH <gregkh@linuxfoundation.org>
wrote:
> > > On Thu, May 24, 2018 at 04:17:12AM -0700, Hugh Dickins wrote:
> > > > Thu, May 24, 2018 at 4:06 AM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org>
> > > > wrote:
> > > >
> > > > > On Thu, May 24, 2018 at 12:50:11PM +0200, Jan Kara wrote:
> > > > > > On Thu 24-05-18 11:38:27, Greg Kroah-Hartman wrote:
> > > > > > > 4.4-stable review patch. If anyone has any objections, please
> > let me
> > > > know.
> > > > > >
> > > > > > Just one objection: Why does stable care about this (and the
> > previous
> > > > > > patch)? I've checked the stable queue and I don't see anything
that
> > > > would
> > > > > > have these patches as a prerequisite. And on their own, they are
> > only
> > > > > > cleanups without substantial gains.
> > > >
> > > > > There's a small gain here:
> > > >
> > > > > > > paralleldd
> > > > > > > 4.4.0
4.4.0
> > > > > > > vanilla
avoidlock
> > > > > > > Amean Elapsd-1 5.28 ( 0.00%) 5.15 (
2.50%)
> > > > > > > Amean Elapsd-4 5.29 ( 0.00%) 5.17 (
2.12%)
> > > > > > > Amean Elapsd-7 5.28 ( 0.00%) 5.18 (
1.78%)
> > > > > > > Amean Elapsd-12 5.20 ( 0.00%) 5.33 (
-2.50%)
> > > > > > > Amean Elapsd-21 5.14 ( 0.00%) 5.21 (
-1.41%)
> > > > > > > Amean Elapsd-30 5.30 ( 0.00%) 5.12 (
3.38%)
> > > > > > > Amean Elapsd-48 5.78 ( 0.00%) 5.42 (
6.21%)
> > > > > > > Amean Elapsd-79 6.78 ( 0.00%) 6.62 (
2.46%)
> > > > > > > Amean Elapsd-110 9.09 ( 0.00%) 8.99 (
1.15%)
> > > > > > > Amean Elapsd-128 10.60 ( 0.00%) 10.43 (
1.66%)
> > > > > > >
> > > > > > > The impact is small but intuitively, it makes sense to avoid
> > > > unnecessary
> > > > > > > calls to lock_page.
> > > >
> > > > > Yes, it's small, but it's marked in the SLES kernel as "needs to
be
> > > > > merged into stable", so obviously it matters to someone :)
> > > >
> > > > Hmm. I had the same reaction to these two as Jan, but assumed that
they
> > > > made applying later patches easier, and didn't take the trouble he
did
> > to
> > > > find that's not so.
> > > >
> > > > I've no wish to be disputatious, but it does seem that the
definition of
> > > > "stable" has changed, and not necessarily for the better, if it's
now a
> > > > home for small gains: I thought we left those to upstream.
> >
> > > This is in the SLES kernel for a reason, and again, it's in the
section
> > > that says "this should be pushed to stable". So if it's good enough
for
> > > the SLES kernel, why isn't it good enough for all users of this kernel
> > > tree?
> >
> > > If you all think it should be dropped in both places, that's fine with
> > > me :)
> >
> > I think they are perfectly fine in SLES: folding in good work is a part
of
> > what distros are about.

> And it's also what stable is for. We have had backports of performance
> improvements in the past, along with lots of other things over the
> years. This is a performance improvement. A tiny one, yes, but getting
> rid of a lock is a good thing, and I picked it up as part of my review
> of what a distro decided was worth adding for their users, as that's a
> huge signal that might be of value to others.

> > But I cannot find anything in stable-kernel-rules.rst that would admit
them
> > - perhaps that's just out of date?

> Nope, that's the list I use to say "no" to. You can't describe
> everything in that file, it's a judgement call.

> > If -stable is to be a compendium of "this looks nice, you might like to
> > include it", so be it: but the rules should then be updated.

> This is a "a bunch of people I trust took it in their kernel, and it's
> been running on zillion of machines for a while and causes no harm and a
> slight benefit, so let's add it!" type of thing. It's not the only
> patch in this series that was like that, but for some reason this one is
> the one the triggered the debate, which is funny to me as this does have
> numbers in it showing that it is an improvement :)

Thank you for looking after the -stable trees: please let me not waste your
time any further. I have no specific objection to the two patches, which
are certainly not egregious offenders. But I do still find the disconnect
between stable-kernel-rules.rst and reality confusing - or perhaps I just
find reality confusing :)

Hugh

\
 
 \ /
  Last update: 2018-05-24 22:02    [W:0.654 / U:0.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site