diff mbox series

[v3,5/7] acpi/ghes, cxl: Refactor work registration functions to support multiple workqueues

Message ID 20241119003915.174386-6-Smita.KoralahalliChannabasappa@amd.com
State New
Headers show
Series acpi/ghes, cper, cxl: Process CXL CPER Protocol errors | expand

Commit Message

Smita Koralahalli Nov. 19, 2024, 12:39 a.m. UTC 
Refactor the work registration and unregistration functions in GHES to
enable reuse across different workqueues. This update lays the foundation
for integrating additional workqueues in the CXL subsystem for better
modularity and code reuse.

Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
---
 drivers/acpi/apei/ghes.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

Comments

Jonathan Cameron Nov. 26, 2024, 3:57 p.m. UTC | #1 
On Tue, 19 Nov 2024 00:39:13 +0000
Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:

> Refactor the work registration and unregistration functions in GHES to
> enable reuse across different workqueues. This update lays the foundation
> for integrating additional workqueues in the CXL subsystem for better
> modularity and code reuse.
> 
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> ---
>  drivers/acpi/apei/ghes.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 082c409707ba..62ffe6eb5503 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -717,26 +717,42 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>  	schedule_work(cxl_cper_work);
>  }
>  
> -int cxl_cper_register_event_work(struct work_struct *work)
> +static int cxl_cper_register_work(struct work_struct **work_ptr,
> +				  spinlock_t *lock,
> +				  struct work_struct *work)

This is a somewhat strange interface.  It doesn't
really do anything particularly useful. I'd be tempted to
just open code this at each call site.


>  {
> -	if (cxl_cper_work)
> +	if (*work_ptr)
>  		return -EINVAL;
>  
> -	guard(spinlock)(&cxl_cper_work_lock);
> -	cxl_cper_work = work;
> +	guard(spinlock)(lock);
> +	*work_ptr = work;
>  	return 0;
>  }
> -EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
>  
> -int cxl_cper_unregister_event_work(struct work_struct *work)
> +static int cxl_cper_unregister_work(struct work_struct **work_ptr,
> +				    spinlock_t *lock,
> +				    struct work_struct *work)
>  {
> -	if (cxl_cper_work != work)
> +	if (*work_ptr != work)
As above.

>  		return -EINVAL;
>  
> -	guard(spinlock)(&cxl_cper_work_lock);
> -	cxl_cper_work = NULL;
> +	guard(spinlock)(lock);
> +	*work_ptr = NULL;
>  	return 0;
>  }
> +
> +int cxl_cper_register_event_work(struct work_struct *work)
> +{
> +	return cxl_cper_register_work(&cxl_cper_work, &cxl_cper_work_lock,
> +				      work);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
> +
> +int cxl_cper_unregister_event_work(struct work_struct *work)
> +{
> +	return cxl_cper_unregister_work(&cxl_cper_work, &cxl_cper_work_lock,
> +					work);
> +}
>  EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_event_work, CXL);
>  
>  int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
Smita Koralahalli Nov. 27, 2024, 7:46 p.m. UTC | #2 
On 11/26/2024 7:57 AM, Jonathan Cameron wrote:
> On Tue, 19 Nov 2024 00:39:13 +0000
> Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
> 
>> Refactor the work registration and unregistration functions in GHES to
>> enable reuse across different workqueues. This update lays the foundation
>> for integrating additional workqueues in the CXL subsystem for better
>> modularity and code reuse.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
>> ---
>>   drivers/acpi/apei/ghes.c | 34 +++++++++++++++++++++++++---------
>>   1 file changed, 25 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 082c409707ba..62ffe6eb5503 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -717,26 +717,42 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>>   	schedule_work(cxl_cper_work);
>>   }
>>   
>> -int cxl_cper_register_event_work(struct work_struct *work)
>> +static int cxl_cper_register_work(struct work_struct **work_ptr,
>> +				  spinlock_t *lock,
>> +				  struct work_struct *work)
> 
> This is a somewhat strange interface.  It doesn't
> really do anything particularly useful. I'd be tempted to
> just open code this at each call site.

Okay I will change.
> 
> 
>>   {
>> -	if (cxl_cper_work)
>> +	if (*work_ptr)
>>   		return -EINVAL;
>>   
>> -	guard(spinlock)(&cxl_cper_work_lock);
>> -	cxl_cper_work = work;
>> +	guard(spinlock)(lock);
>> +	*work_ptr = work;
>>   	return 0;
>>   }
>> -EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
>>   
>> -int cxl_cper_unregister_event_work(struct work_struct *work)
>> +static int cxl_cper_unregister_work(struct work_struct **work_ptr,
>> +				    spinlock_t *lock,
>> +				    struct work_struct *work)
>>   {
>> -	if (cxl_cper_work != work)
>> +	if (*work_ptr != work)
> As above.
okay.

Thanks
Smita
> 
>>   		return -EINVAL;
>>   
>> -	guard(spinlock)(&cxl_cper_work_lock);
>> -	cxl_cper_work = NULL;
>> +	guard(spinlock)(lock);
>> +	*work_ptr = NULL;
>>   	return 0;
>>   }
>> +
>> +int cxl_cper_register_event_work(struct work_struct *work)
>> +{
>> +	return cxl_cper_register_work(&cxl_cper_work, &cxl_cper_work_lock,
>> +				      work);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
>> +
>> +int cxl_cper_unregister_event_work(struct work_struct *work)
>> +{
>> +	return cxl_cper_unregister_work(&cxl_cper_work, &cxl_cper_work_lock,
>> +					work);
>> +}
>>   EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_event_work, CXL);
>>   
>>   int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
>
diff mbox series

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 082c409707ba..62ffe6eb5503 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -717,26 +717,42 @@  static void cxl_cper_post_event(enum cxl_event_type event_type,
 	schedule_work(cxl_cper_work);
 }
 
-int cxl_cper_register_event_work(struct work_struct *work)
+static int cxl_cper_register_work(struct work_struct **work_ptr,
+				  spinlock_t *lock,
+				  struct work_struct *work)
 {
-	if (cxl_cper_work)
+	if (*work_ptr)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_work_lock);
-	cxl_cper_work = work;
+	guard(spinlock)(lock);
+	*work_ptr = work;
 	return 0;
 }
-EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
 
-int cxl_cper_unregister_event_work(struct work_struct *work)
+static int cxl_cper_unregister_work(struct work_struct **work_ptr,
+				    spinlock_t *lock,
+				    struct work_struct *work)
 {
-	if (cxl_cper_work != work)
+	if (*work_ptr != work)
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_work_lock);
-	cxl_cper_work = NULL;
+	guard(spinlock)(lock);
+	*work_ptr = NULL;
 	return 0;
 }
+
+int cxl_cper_register_event_work(struct work_struct *work)
+{
+	return cxl_cper_register_work(&cxl_cper_work, &cxl_cper_work_lock,
+				      work);
+}
+EXPORT_SYMBOL_NS_GPL(cxl_cper_register_event_work, CXL);
+
+int cxl_cper_unregister_event_work(struct work_struct *work)
+{
+	return cxl_cper_unregister_work(&cxl_cper_work, &cxl_cper_work_lock,
+					work);
+}
 EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_event_work, CXL);
 
 int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)