| Message ID | 1424231849-17973-4-git-send-email-benh@kernel.crashing.org |
|---|---|
| State | Accepted |
| Headers | show |
On 02/18/2015 04:57 AM, Benjamin Herrenschmidt wrote: > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > core/timebase.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/core/timebase.c b/core/timebase.c > index 9321373..878f66b 100644 > --- a/core/timebase.c > +++ b/core/timebase.c > @@ -1,3 +1,4 @@ > + > /* Copyright 2013-2014 IBM Corp. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > @@ -40,7 +41,14 @@ static void time_wait_poll(unsigned long duration) > > void time_wait(unsigned long duration) > { > - if (this_cpu() != boot_cpu) > + struct cpu_thread *c = this_cpu(); > + > + if (this_cpu()->lock_depth) { > + time_wait_nopoll(duration); > + return; > + } > + > + if (c != boot_cpu) > time_wait_nopoll(duration); > else > time_wait_poll(duration); > openpower systems call ipmi_wdt_init() which calls ipmi_queue_msg_sync(). and then, they just loop endlessly in time_wait_ms(100) :/ C.
On Wed, 2015-02-18 at 16:12 +0100, Cedric Le Goater wrote: > openpower systems call ipmi_wdt_init() which calls > ipmi_queue_msg_sync(). > > and then, they just loop endlessly in time_wait_ms(100) :/ Yes, it's a design bug in the ipmi code I just noticed. It shouldn't spin with a lock held. I'll try to fix that with Alistair today. Cheers, Ben.
On 02/18/2015 09:58 PM, Benjamin Herrenschmidt wrote: > On Wed, 2015-02-18 at 16:12 +0100, Cedric Le Goater wrote: >> openpower systems call ipmi_wdt_init() which calls >> ipmi_queue_msg_sync(). >> >> and then, they just loop endlessly in time_wait_ms(100) :/ > > Yes, it's a design bug in the ipmi code I just noticed. It shouldn't > spin with a lock held. I'll try to fix that with Alistair today. Thanks. I did not know how to handle it so I just reverted your patch for the moment. C.
Cedric Le Goater <clg@fr.ibm.com> writes: > On 02/18/2015 09:58 PM, Benjamin Herrenschmidt wrote: >> On Wed, 2015-02-18 at 16:12 +0100, Cedric Le Goater wrote: >>> openpower systems call ipmi_wdt_init() which calls >>> ipmi_queue_msg_sync(). >>> >>> and then, they just loop endlessly in time_wait_ms(100) :/ >> >> Yes, it's a design bug in the ipmi code I just noticed. It shouldn't >> spin with a lock held. I'll try to fix that with Alistair today. > > Thanks. I did not know how to handle it so I just reverted your patch > for the moment. Should be fixed in master now.
On 02/19/2015 07:09 AM, Stewart Smith wrote: > Cedric Le Goater <clg@fr.ibm.com> writes: >> On 02/18/2015 09:58 PM, Benjamin Herrenschmidt wrote: >>> On Wed, 2015-02-18 at 16:12 +0100, Cedric Le Goater wrote: >>>> openpower systems call ipmi_wdt_init() which calls >>>> ipmi_queue_msg_sync(). >>>> >>>> and then, they just loop endlessly in time_wait_ms(100) :/ >>> >>> Yes, it's a design bug in the ipmi code I just noticed. It shouldn't >>> spin with a lock held. I'll try to fix that with Alistair today. >> >> Thanks. I did not know how to handle it so I just reverted your patch >> for the moment. > > Should be fixed in master now. > yeah. I see that. thanks. I will give it a try. C.
diff --git a/core/timebase.c b/core/timebase.c index 9321373..878f66b 100644 --- a/core/timebase.c +++ b/core/timebase.c @@ -1,3 +1,4 @@ + /* Copyright 2013-2014 IBM Corp. * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -40,7 +41,14 @@ static void time_wait_poll(unsigned long duration) void time_wait(unsigned long duration) { - if (this_cpu() != boot_cpu) + struct cpu_thread *c = this_cpu(); + + if (this_cpu()->lock_depth) { + time_wait_nopoll(duration); + return; + } + + if (c != boot_cpu) time_wait_nopoll(duration); else time_wait_poll(duration);
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- core/timebase.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)