[PATCH] powerpc: eeh_event: convert semaphore to completion

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(-)

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 8:52 AM Arnd Bergmann [off-list ref] 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?

On Tue, Dec 11, 2018 at 10:18:31AM +1100, Oliver wrote:

On Tue, Dec 11, 2018 at 8:52 AM Arnd Bergmann [off-list ref] wrote:

quoted

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.

quoted

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 Mon, 2018-12-10 at 21:51:57 UTC, Arnd Bergmann 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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/2fea82db113e4422ef5c8e62099088

cheers