All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: lkp@lists.01.org
Subject: Re: [locks] 6d390e4b5d: will-it-scale.per_process_ops -96.6% regression
Date: Tue, 17 Mar 2020 12:07:15 -0400	[thread overview]
Message-ID: <61d6b91e9387969f5dfaba192aee366cc9b310f0.camel@kernel.org> (raw)
In-Reply-To: <6df79609-90eb-2f59-7e86-3532ac309a7a@huawei.com>

[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]

On Tue, 2020-03-17 at 22:05 +0800, yangerkun wrote:
> 
> On 2020/3/17 9:41, yangerkun wrote:
> > 
> > On 2020/3/17 1:26, Linus Torvalds wrote:
> > > On Mon, Mar 16, 2020 at 4:07 AM Jeff Layton <jlayton@kernel.org> wrote:
> > > > 
> > > > +       /*
> > > > +        * If fl_blocker is NULL, it won't be set again as this 
> > > > thread "owns"
> > > > +        * the lock and is the only one that might try to claim the 
> > > > lock.
> > > > +        * Because fl_blocker is explicitly set last during a delete, 
> > > > it's
> > > > +        * safe to locklessly test to see if it's NULL. If it is, 
> > > > then we know
> > > > +        * that no new locks can be inserted into its 
> > > > fl_blocked_requests list,
> > > > +        * and we can therefore avoid doing anything further as long 
> > > > as that
> > > > +        * list is empty.
> > > > +        */
> > > > +       if (!smp_load_acquire(&waiter->fl_blocker) &&
> > > > +           list_empty(&waiter->fl_blocked_requests))
> > > > +               return status;
> > > 
> > > Ack. This looks sane to me now.
> > > 
> > > yangerkun - how did you find the original problem?\
> > 
> > While try to fix CVE-2019-19769, add some log in __locks_wake_up_blocks 
> > help me to rebuild the problem soon. This help me to discern the problem 
> > soon.
> > 
> > > Would you mind using whatever stress test that caused commit
> > > 6d390e4b5d48 ("locks: fix a potential use-after-free problem when
> > > wakeup a waiter") with this patch? And if you did it analytically,
> > > you're a champ and should look at this patch too!
> > 
> > I will try to understand this patch, and if it's looks good to me, will 
> > do the performance test!
> 
> This patch looks good to me, with this patch, the bug '6d390e4b5d48 
> ("locks: fix a potential use-after-free problem when wakeup a waiter")' 
> describes won't happen again. Actually, I find that syzkaller has report 
> this bug before[1], and the log of it can help us to reproduce it with 
> some latency in __locks_wake_up_blocks!
> 
> Also, some ltp testcases describes in [2] pass too with the patch!
> 
> For performance test, I have try to understand will-it-scale/lkp, but it 
> seem a little complex to me, and may need some more time. So, Rong Chen, 
> can you help to do this? Or the results may come a little later...
> 
> Thanks,
> ----
> [1] https://syzkaller.appspot.com/bug?extid=922689db06e57b69c240
> [2] https://lkml.org/lkml/2020/3/11/578

Thanks yangerkun. Let me know if you want to add your Reviewed-by tag.

Cheers,
-- 
Jeff Layton <jlayton@kernel.org>

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton@kernel.org>
To: yangerkun <yangerkun@huawei.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: NeilBrown <neilb@suse.de>,
	kernel test robot <rong.a.chen@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	lkp@lists.01.org, Bruce Fields <bfields@fieldses.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [locks] 6d390e4b5d: will-it-scale.per_process_ops -96.6% regression
Date: Tue, 17 Mar 2020 12:07:15 -0400	[thread overview]
Message-ID: <61d6b91e9387969f5dfaba192aee366cc9b310f0.camel@kernel.org> (raw)
In-Reply-To: <6df79609-90eb-2f59-7e86-3532ac309a7a@huawei.com>

On Tue, 2020-03-17 at 22:05 +0800, yangerkun wrote:
> 
> On 2020/3/17 9:41, yangerkun wrote:
> > 
> > On 2020/3/17 1:26, Linus Torvalds wrote:
> > > On Mon, Mar 16, 2020 at 4:07 AM Jeff Layton <jlayton@kernel.org> wrote:
> > > > 
> > > > +       /*
> > > > +        * If fl_blocker is NULL, it won't be set again as this 
> > > > thread "owns"
> > > > +        * the lock and is the only one that might try to claim the 
> > > > lock.
> > > > +        * Because fl_blocker is explicitly set last during a delete, 
> > > > it's
> > > > +        * safe to locklessly test to see if it's NULL. If it is, 
> > > > then we know
> > > > +        * that no new locks can be inserted into its 
> > > > fl_blocked_requests list,
> > > > +        * and we can therefore avoid doing anything further as long 
> > > > as that
> > > > +        * list is empty.
> > > > +        */
> > > > +       if (!smp_load_acquire(&waiter->fl_blocker) &&
> > > > +           list_empty(&waiter->fl_blocked_requests))
> > > > +               return status;
> > > 
> > > Ack. This looks sane to me now.
> > > 
> > > yangerkun - how did you find the original problem?\
> > 
> > While try to fix CVE-2019-19769, add some log in __locks_wake_up_blocks 
> > help me to rebuild the problem soon. This help me to discern the problem 
> > soon.
> > 
> > > Would you mind using whatever stress test that caused commit
> > > 6d390e4b5d48 ("locks: fix a potential use-after-free problem when
> > > wakeup a waiter") with this patch? And if you did it analytically,
> > > you're a champ and should look at this patch too!
> > 
> > I will try to understand this patch, and if it's looks good to me, will 
> > do the performance test!
> 
> This patch looks good to me, with this patch, the bug '6d390e4b5d48 
> ("locks: fix a potential use-after-free problem when wakeup a waiter")' 
> describes won't happen again. Actually, I find that syzkaller has report 
> this bug before[1], and the log of it can help us to reproduce it with 
> some latency in __locks_wake_up_blocks!
> 
> Also, some ltp testcases describes in [2] pass too with the patch!
> 
> For performance test, I have try to understand will-it-scale/lkp, but it 
> seem a little complex to me, and may need some more time. So, Rong Chen, 
> can you help to do this? Or the results may come a little later...
> 
> Thanks,
> ----
> [1] https://syzkaller.appspot.com/bug?extid=922689db06e57b69c240
> [2] https://lkml.org/lkml/2020/3/11/578

Thanks yangerkun. Let me know if you want to add your Reviewed-by tag.

Cheers,
-- 
Jeff Layton <jlayton@kernel.org>


  reply	other threads:[~2020-03-17 16:07 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-08 14:03 [locks] 6d390e4b5d: will-it-scale.per_process_ops -96.6% regression kernel test robot
2020-03-08 14:03 ` kernel test robot
2020-03-09 14:36 ` Jeff Layton
2020-03-09 14:36   ` Jeff Layton
2020-03-09 15:52   ` Linus Torvalds
2020-03-09 15:52     ` Linus Torvalds
2020-03-09 17:22     ` Jeff Layton
2020-03-09 17:22       ` Jeff Layton
2020-03-09 19:09       ` Jeff Layton
2020-03-09 19:09         ` Jeff Layton
2020-03-09 19:53         ` Jeff Layton
2020-03-09 19:53           ` Jeff Layton
2020-03-09 21:42         ` NeilBrown
2020-03-09 21:42           ` NeilBrown
2020-03-09 21:58           ` Jeff Layton
2020-03-09 21:58             ` Jeff Layton
2020-03-10  7:52             ` kernel test robot
2020-03-10  7:52               ` kernel test robot
2020-03-09 22:11           ` Jeff Layton
2020-03-09 22:11             ` Jeff Layton
2020-03-10  3:24             ` yangerkun
2020-03-10  3:24               ` yangerkun
2020-03-10  7:54               ` kernel test robot
2020-03-10  7:54                 ` kernel test robot
2020-03-10 12:52               ` Jeff Layton
2020-03-10 12:52                 ` Jeff Layton
2020-03-10 14:18                 ` yangerkun
2020-03-10 14:18                   ` yangerkun
2020-03-10 15:06                   ` Jeff Layton
2020-03-10 15:06                     ` Jeff Layton
2020-03-10 17:27                 ` Jeff Layton
2020-03-10 17:27                   ` Jeff Layton
2020-03-10 21:01                   ` NeilBrown
2020-03-10 21:01                     ` NeilBrown
2020-03-10 21:14                     ` Jeff Layton
2020-03-10 21:14                       ` Jeff Layton
2020-03-10 21:21                       ` NeilBrown
2020-03-10 21:21                         ` NeilBrown
2020-03-10 21:47                         ` Linus Torvalds
2020-03-10 21:47                           ` Linus Torvalds
2020-03-10 22:07                           ` Jeff Layton
2020-03-10 22:07                             ` Jeff Layton
2020-03-10 22:31                             ` Linus Torvalds
2020-03-10 22:31                               ` Linus Torvalds
2020-03-11 22:22                               ` NeilBrown
2020-03-11 22:22                                 ` NeilBrown
2020-03-12  0:38                                 ` Linus Torvalds
2020-03-12  0:38                                   ` Linus Torvalds
2020-03-12  4:42                                   ` NeilBrown
2020-03-12  4:42                                     ` NeilBrown
2020-03-12 12:31                                     ` Jeff Layton
2020-03-12 12:31                                       ` Jeff Layton
2020-03-12 22:19                                       ` NeilBrown
2020-03-12 22:19                                         ` NeilBrown
2020-03-14  1:11                                         ` Jeff Layton
2020-03-14  1:11                                           ` Jeff Layton
2020-03-12 16:07                                     ` Linus Torvalds
2020-03-12 16:07                                       ` Linus Torvalds
2020-03-14  1:31                                       ` Jeff Layton
2020-03-14  1:31                                         ` Jeff Layton
2020-03-14  2:31                                         ` NeilBrown
2020-03-14  2:31                                           ` NeilBrown
2020-03-14 15:58                                           ` Linus Torvalds
2020-03-14 15:58                                             ` Linus Torvalds
2020-03-15 13:54                                             ` Jeff Layton
2020-03-15 13:54                                               ` Jeff Layton
2020-03-16  5:06                                               ` NeilBrown
2020-03-16  5:06                                                 ` NeilBrown
2020-03-16 11:07                                                 ` Jeff Layton
2020-03-16 11:07                                                   ` Jeff Layton
2020-03-16 17:26                                                   ` Linus Torvalds
2020-03-16 17:26                                                     ` Linus Torvalds
2020-03-17  1:41                                                     ` yangerkun
2020-03-17  1:41                                                       ` yangerkun
2020-03-17 14:05                                                       ` yangerkun
2020-03-17 14:05                                                         ` yangerkun
2020-03-17 16:07                                                         ` Jeff Layton [this message]
2020-03-17 16:07                                                           ` Jeff Layton
2020-03-18  1:09                                                           ` yangerkun
2020-03-18  1:09                                                             ` yangerkun
2020-03-19 17:51                                                     ` Jeff Layton
2020-03-19 17:51                                                       ` Jeff Layton
2020-03-19 19:23                                                       ` Linus Torvalds
2020-03-19 19:23                                                         ` Linus Torvalds
2020-03-19 19:24                                                         ` Jeff Layton
2020-03-19 19:24                                                           ` Jeff Layton
2020-03-19 19:35                                                           ` Linus Torvalds
2020-03-19 19:35                                                             ` Linus Torvalds
2020-03-19 20:10                                                             ` Jeff Layton
2020-03-19 20:10                                                               ` Jeff Layton
2020-03-16 22:45                                                   ` NeilBrown
2020-03-16 22:45                                                     ` NeilBrown
2020-03-17 15:59                                                     ` Jeff Layton
2020-03-17 15:59                                                       ` Jeff Layton
2020-03-17 21:27                                                       ` NeilBrown
2020-03-17 21:27                                                         ` NeilBrown
2020-03-18  5:12                                                   ` kernel test robot
2020-03-18  5:12                                                     ` kernel test robot
2020-03-16  4:26                                             ` NeilBrown
2020-03-16  4:26                                               ` NeilBrown
2020-03-11  1:57                     ` yangerkun
2020-03-11  1:57                       ` yangerkun
2020-03-11 12:52                       ` Jeff Layton
2020-03-11 12:52                         ` Jeff Layton
2020-03-11 13:26                         ` yangerkun
2020-03-11 13:26                           ` yangerkun
2020-03-11 22:15                       ` NeilBrown
2020-03-11 22:15                         ` NeilBrown
2020-03-10  7:50           ` kernel test robot
2020-03-10  7:50             ` kernel test robot

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=61d6b91e9387969f5dfaba192aee366cc9b310f0.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=lkp@lists.01.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.