Re: [PATCH 1/1] acpi/ec.c: Move call to kref_get() to under the mutex_lock(&ec->mutex)

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Monday, November 25, 2019 12:03:18 PM CET kc27041980@gmail.com wrote:
> From: KC27041980 <kc27041980@gmail.com>
> 
> Move call to kref_get() to under the mutex_lock(&ec->mutex) as this
> will remove any delete race scenarios.
> 
> Signed-off-by: KC27041980 <kc27041980@gmail.com>
> ---
>  drivers/acpi/ec.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index da1e5c5..9c1bd57 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1073,8 +1073,12 @@ acpi_ec_get_query_handler_by_value(struct acpi_ec *ec, u8 value)
>  			break;
>  		}
>  	}
> +
> +	if (found)
> +		acpi_ec_get_query_handler(handler);
>  	mutex_unlock(&ec->mutex);
> -	return found ? acpi_ec_get_query_handler(handler) : NULL;
> +
> +	return found ? handler : NULL;
>  }
>  
>  static void acpi_ec_query_handler_release(struct kref *kref)
> 

Well, what about the appended patch instead?

---
 drivers/acpi/ec.c |   16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -1053,28 +1053,20 @@ void acpi_ec_unblock_transactions(void)
                                 Event Management
    -------------------------------------------------------------------------- */
 static struct acpi_ec_query_handler *
-acpi_ec_get_query_handler(struct acpi_ec_query_handler *handler)
-{
-	if (handler)
-		kref_get(&handler->kref);
-	return handler;
-}
-
-static struct acpi_ec_query_handler *
 acpi_ec_get_query_handler_by_value(struct acpi_ec *ec, u8 value)
 {
 	struct acpi_ec_query_handler *handler;
-	bool found = false;
 
 	mutex_lock(&ec->mutex);
 	list_for_each_entry(handler, &ec->list, node) {
 		if (value == handler->query_bit) {
-			found = true;
-			break;
+			kref_get(&handler->kref);
+			mutex_unlock(&ec->mutex);
+			return handler;
 		}
 	}
 	mutex_unlock(&ec->mutex);
-	return found ? acpi_ec_get_query_handler(handler) : NULL;
+	return NULL;
 }
 
 static void acpi_ec_query_handler_release(struct kref *kref)