| Message ID | 20181210215203.1936171-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | 2fea82db113e4422ef5c8e6209908806e5fb6b63 |
| Headers | show |
| Series | powerpc: eeh_event: convert semaphore to completion | expand |
On Tue, Dec 11, 2018 at 8:52 AM Arnd Bergmann <arnd@arndb.de> wrote: > > For this use case, completions and semaphores are equivalent, > but semaphores are an awkward interface that should generally > be avoided, so use the completion instead. IIRC Sam has been reworking the locking used inside of EEH so this is probably going to clash with his changes. Converting to a completion is probably a good idea, but we might want to do it as a part of his series since it's going to collide with this anyway. Sam, what do you think? > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > arch/powerpc/kernel/eeh_event.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c > index 61c9356bf9c9..227e57f980df 100644 > --- a/arch/powerpc/kernel/eeh_event.c > +++ b/arch/powerpc/kernel/eeh_event.c > @@ -35,7 +35,7 @@ > */ > > static DEFINE_SPINLOCK(eeh_eventlist_lock); > -static struct semaphore eeh_eventlist_sem; > +static DECLARE_COMPLETION(eeh_eventlist_event); > static LIST_HEAD(eeh_eventlist); > > /** > @@ -55,7 +55,7 @@ static int eeh_event_handler(void * dummy) > struct eeh_pe *pe; > > while (!kthread_should_stop()) { > - if (down_interruptible(&eeh_eventlist_sem)) > + if (wait_for_completion_interruptible(&eeh_eventlist_event)) > break; > > /* Fetch EEH event from the queue */ > @@ -102,9 +102,6 @@ int eeh_event_init(void) > struct task_struct *t; > int ret = 0; > > - /* Initialize semaphore */ > - sema_init(&eeh_eventlist_sem, 0); > - > t = kthread_run(eeh_event_handler, NULL, "eehd"); > if (IS_ERR(t)) { > ret = PTR_ERR(t); > @@ -142,7 +139,7 @@ int eeh_send_failure_event(struct eeh_pe *pe) > spin_unlock_irqrestore(&eeh_eventlist_lock, flags); > > /* For EEH deamon to knick in */ > - up(&eeh_eventlist_sem); > + complete(&eeh_eventlist_event); > > return 0; > } > -- > 2.20.0 >
On Tue, Dec 11, 2018 at 10:18:31AM +1100, Oliver wrote: > On Tue, Dec 11, 2018 at 8:52 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > For this use case, completions and semaphores are equivalent, > > but semaphores are an awkward interface that should generally > > be avoided, so use the completion instead. > > IIRC Sam has been reworking the locking used inside of EEH so this is > probably going to clash with his changes. Converting to a completion > is probably a good idea, but we might want to do it as a part of his > series since it's going to collide with this anyway. > > Sam, what do you think? It's such a small change, I don't think it will cause any problems for the rework. Anyway it seems like a good change, so I'd prefer to see it go in :-) Cheers, Sam. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > arch/powerpc/kernel/eeh_event.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c > > index 61c9356bf9c9..227e57f980df 100644 > > --- a/arch/powerpc/kernel/eeh_event.c > > +++ b/arch/powerpc/kernel/eeh_event.c > > @@ -35,7 +35,7 @@ > > */ > > > > static DEFINE_SPINLOCK(eeh_eventlist_lock); > > -static struct semaphore eeh_eventlist_sem; > > +static DECLARE_COMPLETION(eeh_eventlist_event); > > static LIST_HEAD(eeh_eventlist); > > > > /** > > @@ -55,7 +55,7 @@ static int eeh_event_handler(void * dummy) > > struct eeh_pe *pe; > > > > while (!kthread_should_stop()) { > > - if (down_interruptible(&eeh_eventlist_sem)) > > + if (wait_for_completion_interruptible(&eeh_eventlist_event)) > > break; > > > > /* Fetch EEH event from the queue */ > > @@ -102,9 +102,6 @@ int eeh_event_init(void) > > struct task_struct *t; > > int ret = 0; > > > > - /* Initialize semaphore */ > > - sema_init(&eeh_eventlist_sem, 0); > > - > > t = kthread_run(eeh_event_handler, NULL, "eehd"); > > if (IS_ERR(t)) { > > ret = PTR_ERR(t); > > @@ -142,7 +139,7 @@ int eeh_send_failure_event(struct eeh_pe *pe) > > spin_unlock_irqrestore(&eeh_eventlist_lock, flags); > > > > /* For EEH deamon to knick in */ > > - up(&eeh_eventlist_sem); > > + complete(&eeh_eventlist_event); > > > > return 0; > > } > > -- > > 2.20.0 > > >
diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c index 61c9356bf9c9..227e57f980df 100644 --- a/arch/powerpc/kernel/eeh_event.c +++ b/arch/powerpc/kernel/eeh_event.c @@ -35,7 +35,7 @@ */ static DEFINE_SPINLOCK(eeh_eventlist_lock); -static struct semaphore eeh_eventlist_sem; +static DECLARE_COMPLETION(eeh_eventlist_event); static LIST_HEAD(eeh_eventlist); /** @@ -55,7 +55,7 @@ static int eeh_event_handler(void * dummy) struct eeh_pe *pe; while (!kthread_should_stop()) { - if (down_interruptible(&eeh_eventlist_sem)) + if (wait_for_completion_interruptible(&eeh_eventlist_event)) break; /* Fetch EEH event from the queue */ @@ -102,9 +102,6 @@ int eeh_event_init(void) struct task_struct *t; int ret = 0; - /* Initialize semaphore */ - sema_init(&eeh_eventlist_sem, 0); - t = kthread_run(eeh_event_handler, NULL, "eehd"); if (IS_ERR(t)) { ret = PTR_ERR(t); @@ -142,7 +139,7 @@ int eeh_send_failure_event(struct eeh_pe *pe) spin_unlock_irqrestore(&eeh_eventlist_lock, flags); /* For EEH deamon to knick in */ - up(&eeh_eventlist_sem); + complete(&eeh_eventlist_event); return 0; }
For this use case, completions and semaphores are equivalent, but semaphores are an awkward interface that should generally be avoided, so use the completion instead. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/powerpc/kernel/eeh_event.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) -- 2.20.0