Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Thu, 2017-04-13 at 13:31 +0200, Borislav Petkov wrote: > On Thu, Apr 13, 2017 at 12:29:25AM +0200, Borislav Petkov wrote: > > On Wed, Apr 12, 2017 at 03:26:19PM -0700, Luck, Tony wrote: > > > We can futz with that and have them specify which chain (or both) > > > that they want to be added to.

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Thu, Apr 13, 2017 at 02:12:16PM +0200, Borislav Petkov wrote: > On Thu, Apr 13, 2017 at 01:31:59PM +0200, Borislav Petkov wrote: > > @@ -321,18 +321,8 @@ static void __print_mce(struct mce *m) > > > > static void print_mce(struct mce *m) > > { > > - int ret = 0; > > - > >

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 04:45:59PM -0700, Paul E. McKenney wrote: > On Wed, Apr 12, 2017 at 03:42:32PM -0700, Paul E. McKenney wrote: > > On Wed, Apr 12, 2017 at 11:50:45PM +0200, Thomas Gleixner wrote: > > > On Wed, 12 Apr 2017, Borislav Petkov wrote: > > > > > > > On Wed, Apr 12, 2017 at

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Thu, Apr 13, 2017 at 01:31:59PM +0200, Borislav Petkov wrote: > @@ -321,18 +321,8 @@ static void __print_mce(struct mce *m) > > static void print_mce(struct mce *m) > { > - int ret = 0; > - > __print_mce(m); > - > - /* > - * Print out human-readable details about the MCE

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Thu, Apr 13, 2017 at 12:29:25AM +0200, Borislav Petkov wrote: > On Wed, Apr 12, 2017 at 03:26:19PM -0700, Luck, Tony wrote: > > We can futz with that and have them specify which chain (or both) > > that they want to be added to. > > Well, I didn't want the atomic chain to be a notifier because

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 03:42:32PM -0700, Paul E. McKenney wrote: > On Wed, Apr 12, 2017 at 11:50:45PM +0200, Thomas Gleixner wrote: > > On Wed, 12 Apr 2017, Borislav Petkov wrote: > > > > > On Wed, Apr 12, 2017 at 08:27:05PM +, Verma, Vishal L wrote: > > > > But isn't the atomic notifier

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 03:26:19PM -0700, Luck, Tony wrote: > We can futz with that and have them specify which chain (or both) > that they want to be added to. Well, I didn't want the atomic chain to be a notifier because we can keep it simple and non-blocking. Only the process context one will

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Thu, Apr 13, 2017 at 12:16:39AM +0200, Borislav Petkov wrote: > ... and it is midnight here so I could be talking crap but we probably > should really split the reporting "chain" into two: This shouldn't be too painful. Users ask to be put on the chain via the wrapper: void

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 11:47:49PM +0200, Borislav Petkov wrote: > so we need to think about something better to handle events down the > whole chain. Maybe route events from the atomic path to the blocking > path where the sleeping notifier callbacks can sleep as much as they > want to... ...

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, 12 Apr 2017, Borislav Petkov wrote: > On Wed, Apr 12, 2017 at 08:27:05PM +, Verma, Vishal L wrote: > > But isn't the atomic notifier call chain always called in atomic > > context? > > No, it isn't. We're calling it in normal process context in > mce_gen_pool_process() too. > > So

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 08:27:05PM +, Verma, Vishal L wrote: > But isn't the atomic notifier call chain always called in atomic > context? No, it isn't. We're calling it in normal process context in mce_gen_pool_process() too. So this early exit will avoid any sleeping in atomic context. And

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, 12 Apr 2017, Dan Williams wrote: > On Wed, Apr 12, 2017 at 1:52 PM, Luck, Tony wrote: > > On Wed, Apr 12, 2017 at 01:27:05PM -0700, Verma, Vishal L wrote: > >> > /* We only care about memory errors */ > >> > if (!(mce->status & MCACOD)) > >> >

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 01:27:05PM -0700, Verma, Vishal L wrote: > >   /* We only care about memory errors */ > >   if (!(mce->status & MCACOD)) > >   return NOTIFY_DONE; N.B. that isn't a valid test that this is a memory error. You need if (!(m->status & 0xef80) ==

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, 2017-04-12 at 22:22 +0200, Borislav Petkov wrote: > On Wed, Apr 12, 2017 at 01:59:03PM -0600, Vishal Verma wrote: > > I don't think we can do anything about the panic path errors. > > Then the fix should be a lot easier: > > --- > diff --git a/drivers/acpi/nfit/mce.c

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Wed, Apr 12, 2017 at 01:59:03PM -0600, Vishal Verma wrote: > I don't think we can do anything about the panic path errors. Then the fix should be a lot easier: --- diff --git a/drivers/acpi/nfit/mce.c b/drivers/acpi/nfit/mce.c index 3ba1c3472cf9..44c092ec2ac9 100644 ---

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On 04/12, Borislav Petkov wrote: > On Tue, Apr 11, 2017 at 04:44:57PM -0600, Vishal Verma wrote: > > The NFIT MCE handler callback (for handling media errors on NVDIMMs) > > takes a mutex to add the location of a memory error to a list. But since > > the notifier call chain for machine checks

Re: [RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

On Tue, Apr 11, 2017 at 04:44:57PM -0600, Vishal Verma wrote: > The NFIT MCE handler callback (for handling media errors on NVDIMMs) > takes a mutex to add the location of a memory error to a list. But since > the notifier call chain for machine checks (x86_mce_decoder_chain) is > atomic, we get a

[RFC PATCH] x86, mce: change the mce notifier to 'blocking' from 'atomic'

The NFIT MCE handler callback (for handling media errors on NVDIMMs) takes a mutex to add the location of a memory error to a list. But since the notifier call chain for machine checks (x86_mce_decoder_chain) is atomic, we get a lockdep splat like: BUG: sleeping function called from invalid