All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: Loadavg accounting error on arm64
Date: Mon, 16 Nov 2020 17:41:20 +0000	[thread overview]
Message-ID: <20201116174119.GB30723@willie-the-truck> (raw)
In-Reply-To: <20201116172444.GV3371@techsingularity.net>

On Mon, Nov 16, 2020 at 05:24:44PM +0000, Mel Gorman wrote:
> On Mon, Nov 16, 2020 at 05:49:28PM +0100, Peter Zijlstra wrote:
> > > So while we might be able to avoid a smp_rmb() before the read of
> > > sched_contributes_to_load and rely on p->on_cpu ordering there,
> > > we may still need a smp_wmb() after nr_interruptible() increments
> > > instead of waiting until the smp_store_release() is hit while a task
> > > is scheduling. That would be a real memory barrier on arm64 and a plain
> > > compiler barrier on x86-64.
> > 
> 
> Wish I read this before sending the changelog
> 
> > I'm mighty confused by your words here; and the patch below. What actual
> > scenario are you worried about?
> > 
> 
> The wrong one apparently. Even if the IRQ is released, the IPI would
> deliver to the CPU that should observe the correct value or take the
> other path when smp_cond_load_acquire(&p->on_cpu, !VAL) waits for the
> schedule to finish so I'm now both confused and wondering why smp_wmb
> made a difference at all.

Probably still worth trying Peter's hack to pad the bitfields though, as I
think that's still a plausible issue (and one which would appear to be
fixed by that smp_wmb() too).

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: Loadavg accounting error on arm64
Date: Mon, 16 Nov 2020 17:41:20 +0000	[thread overview]
Message-ID: <20201116174119.GB30723@willie-the-truck> (raw)
In-Reply-To: <20201116172444.GV3371@techsingularity.net>

On Mon, Nov 16, 2020 at 05:24:44PM +0000, Mel Gorman wrote:
> On Mon, Nov 16, 2020 at 05:49:28PM +0100, Peter Zijlstra wrote:
> > > So while we might be able to avoid a smp_rmb() before the read of
> > > sched_contributes_to_load and rely on p->on_cpu ordering there,
> > > we may still need a smp_wmb() after nr_interruptible() increments
> > > instead of waiting until the smp_store_release() is hit while a task
> > > is scheduling. That would be a real memory barrier on arm64 and a plain
> > > compiler barrier on x86-64.
> > 
> 
> Wish I read this before sending the changelog
> 
> > I'm mighty confused by your words here; and the patch below. What actual
> > scenario are you worried about?
> > 
> 
> The wrong one apparently. Even if the IRQ is released, the IPI would
> deliver to the CPU that should observe the correct value or take the
> other path when smp_cond_load_acquire(&p->on_cpu, !VAL) waits for the
> schedule to finish so I'm now both confused and wondering why smp_wmb
> made a difference at all.

Probably still worth trying Peter's hack to pad the bitfields though, as I
think that's still a plausible issue (and one which would appear to be
fixed by that smp_wmb() too).

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-11-16 17:42 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16  9:10 Loadavg accounting error on arm64 Mel Gorman
2020-11-16  9:10 ` Mel Gorman
2020-11-16 11:49 ` Mel Gorman
2020-11-16 11:49   ` Mel Gorman
2020-11-16 12:00   ` Mel Gorman
2020-11-16 12:00     ` Mel Gorman
2020-11-16 12:53   ` Peter Zijlstra
2020-11-16 12:53     ` Peter Zijlstra
2020-11-16 12:58     ` Peter Zijlstra
2020-11-16 12:58       ` Peter Zijlstra
2020-11-16 15:29       ` Mel Gorman
2020-11-16 15:29         ` Mel Gorman
2020-11-16 16:42         ` Mel Gorman
2020-11-16 16:42           ` Mel Gorman
2020-11-16 16:49         ` Peter Zijlstra
2020-11-16 16:49           ` Peter Zijlstra
2020-11-16 17:24           ` Mel Gorman
2020-11-16 17:24             ` Mel Gorman
2020-11-16 17:41             ` Will Deacon [this message]
2020-11-16 17:41               ` Will Deacon
2020-11-16 12:46 ` Peter Zijlstra
2020-11-16 12:46   ` Peter Zijlstra
2020-11-16 12:58   ` Mel Gorman
2020-11-16 12:58     ` Mel Gorman
2020-11-16 13:11 ` Will Deacon
2020-11-16 13:11   ` Will Deacon
2020-11-16 13:37   ` Mel Gorman
2020-11-16 13:37     ` Mel Gorman
2020-11-16 14:20     ` Peter Zijlstra
2020-11-16 14:20       ` Peter Zijlstra
2020-11-16 15:52       ` Mel Gorman
2020-11-16 15:52         ` Mel Gorman
2020-11-16 16:54         ` Peter Zijlstra
2020-11-16 16:54           ` Peter Zijlstra
2020-11-16 17:16           ` Mel Gorman
2020-11-16 17:16             ` Mel Gorman
2020-11-16 19:31       ` Mel Gorman
2020-11-16 19:31         ` Mel Gorman
2020-11-17  8:30         ` [PATCH] sched: Fix data-race in wakeup Peter Zijlstra
2020-11-17  8:30           ` Peter Zijlstra
2020-11-17  9:15           ` Will Deacon
2020-11-17  9:15             ` Will Deacon
2020-11-17  9:29             ` Peter Zijlstra
2020-11-17  9:29               ` Peter Zijlstra
2020-11-17  9:46               ` Peter Zijlstra
2020-11-17  9:46                 ` Peter Zijlstra
2020-11-17 10:36                 ` Will Deacon
2020-11-17 10:36                   ` Will Deacon
2020-11-17 12:52                 ` Valentin Schneider
2020-11-17 12:52                   ` Valentin Schneider
2020-11-17 15:37                   ` Valentin Schneider
2020-11-17 15:37                     ` Valentin Schneider
2020-11-17 16:13                     ` Peter Zijlstra
2020-11-17 16:13                       ` Peter Zijlstra
2020-11-17 19:32                       ` Valentin Schneider
2020-11-17 19:32                         ` Valentin Schneider
2020-11-18  8:05                         ` Peter Zijlstra
2020-11-18  8:05                           ` Peter Zijlstra
2020-11-18  9:51                           ` Valentin Schneider
2020-11-18  9:51                             ` Valentin Schneider
2020-11-18 13:33               ` Marco Elver
2020-11-18 13:33                 ` Marco Elver
2020-11-17  9:38           ` [PATCH] sched: Fix rq->nr_iowait ordering Peter Zijlstra
2020-11-17  9:38             ` Peter Zijlstra
2020-11-17 11:43             ` Mel Gorman
2020-11-17 11:43               ` Mel Gorman
2020-11-19  9:55             ` [tip: sched/urgent] " tip-bot2 for Peter Zijlstra
2020-11-17 12:40           ` [PATCH] sched: Fix data-race in wakeup Mel Gorman
2020-11-17 12:40             ` Mel Gorman
2020-11-19  9:55           ` [tip: sched/urgent] " tip-bot2 for Peter Zijlstra

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=20201116174119.GB30723@willie-the-truck \
    --to=will@kernel.org \
    --cc=dave@stgolabs.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@techsingularity.net \
    --cc=peterz@infradead.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.