LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] genirq: Simplify the check for __irq_get_desc_lock()
@ 2024-05-06 12:50 Jinjie Ruan
  2024-05-06 17:55 ` Thomas Gleixner
  0 siblings, 1 reply; 4+ messages in thread
From: Jinjie Ruan @ 2024-05-06 12:50 UTC (permalink / raw
  To: tglx, linux-kernel; +Cc: ruanjinjie

If it set "_IRQ_DESC_PERCPU" in "check" but the desc is not percpu, or if
the desc is percpu but it not set "_IRQ_DESC_PERCPU" in "check", it both
return NULL, so simplify the check in __irq_get_desc_lock() with "!=".

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 kernel/irq/irqdesc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 88ac3652fcf2..6c52deb134b9 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -882,11 +882,7 @@ __irq_get_desc_lock(unsigned int irq, unsigned long *flags, bool bus,
 
 	if (desc) {
 		if (check & _IRQ_DESC_CHECK) {
-			if ((check & _IRQ_DESC_PERCPU) &&
-			    !irq_settings_is_per_cpu_devid(desc))
-				return NULL;
-
-			if (!(check & _IRQ_DESC_PERCPU) &&
+			if (!!(check & _IRQ_DESC_PERCPU) !=
 			    irq_settings_is_per_cpu_devid(desc))
 				return NULL;
 		}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] genirq: Simplify the check for __irq_get_desc_lock()
  2024-05-06 12:50 [PATCH] genirq: Simplify the check for __irq_get_desc_lock() Jinjie Ruan
@ 2024-05-06 17:55 ` Thomas Gleixner
  2024-05-07  1:16   ` Jinjie Ruan
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2024-05-06 17:55 UTC (permalink / raw
  To: Jinjie Ruan, linux-kernel; +Cc: ruanjinjie

On Mon, May 06 2024 at 20:50, Jinjie Ruan wrote:

> If it set "_IRQ_DESC_PERCPU" in "check" but the desc is not percpu, or if
> the desc is percpu but it not set "_IRQ_DESC_PERCPU" in "check", it both
> return NULL, so simplify the check in __irq_get_desc_lock() with "!=".

What is exactly simplified here?

> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>  kernel/irq/irqdesc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index 88ac3652fcf2..6c52deb134b9 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -882,11 +882,7 @@ __irq_get_desc_lock(unsigned int irq, unsigned long *flags, bool bus,
>  
>  	if (desc) {
>  		if (check & _IRQ_DESC_CHECK) {
> -			if ((check & _IRQ_DESC_PERCPU) &&
> -			    !irq_settings_is_per_cpu_devid(desc))
> -				return NULL;
> -
> -			if (!(check & _IRQ_DESC_PERCPU) &&
> +			if (!!(check & _IRQ_DESC_PERCPU) !=
>  			    irq_settings_is_per_cpu_devid(desc))
>  				return NULL;

The existing code is readable and obvious. This is not.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] genirq: Simplify the check for __irq_get_desc_lock()
  2024-05-06 17:55 ` Thomas Gleixner
@ 2024-05-07  1:16   ` Jinjie Ruan
  2024-05-07 10:20     ` Thomas Gleixner
  0 siblings, 1 reply; 4+ messages in thread
From: Jinjie Ruan @ 2024-05-07  1:16 UTC (permalink / raw
  To: Thomas Gleixner, linux-kernel



On 2024/5/7 1:55, Thomas Gleixner wrote:
> On Mon, May 06 2024 at 20:50, Jinjie Ruan wrote:
> 
>> If it set "_IRQ_DESC_PERCPU" in "check" but the desc is not percpu, or if
>> the desc is percpu but it not set "_IRQ_DESC_PERCPU" in "check", it both
>> return NULL, so simplify the check in __irq_get_desc_lock() with "!=".
> 
> What is exactly simplified here?
> 
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  kernel/irq/irqdesc.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>> index 88ac3652fcf2..6c52deb134b9 100644
>> --- a/kernel/irq/irqdesc.c
>> +++ b/kernel/irq/irqdesc.c
>> @@ -882,11 +882,7 @@ __irq_get_desc_lock(unsigned int irq, unsigned long *flags, bool bus,
>>  
>>  	if (desc) {
>>  		if (check & _IRQ_DESC_CHECK) {
>> -			if ((check & _IRQ_DESC_PERCPU) &&
>> -			    !irq_settings_is_per_cpu_devid(desc))
>> -				return NULL;
>> -
>> -			if (!(check & _IRQ_DESC_PERCPU) &&
>> +			if (!!(check & _IRQ_DESC_PERCPU) !=
>>  			    irq_settings_is_per_cpu_devid(desc))
>>  				return NULL;
> 
> The existing code is readable and obvious. This is not.

Thank you for your review. The existing code is indeed clear, but it
seems that both judgments are checking whether the percpu flags are
consistent.

> 
> Thanks,
> 
>         tglx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] genirq: Simplify the check for __irq_get_desc_lock()
  2024-05-07  1:16   ` Jinjie Ruan
@ 2024-05-07 10:20     ` Thomas Gleixner
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Gleixner @ 2024-05-07 10:20 UTC (permalink / raw
  To: Jinjie Ruan, linux-kernel

On Tue, May 07 2024 at 09:16, Jinjie Ruan wrote:
> On 2024/5/7 1:55, Thomas Gleixner wrote:
>>>  	if (desc) {
>>>  		if (check & _IRQ_DESC_CHECK) {
>>> -			if ((check & _IRQ_DESC_PERCPU) &&
>>> -			    !irq_settings_is_per_cpu_devid(desc))
>>> -				return NULL;
>>> -
>>> -			if (!(check & _IRQ_DESC_PERCPU) &&
>>> +			if (!!(check & _IRQ_DESC_PERCPU) !=
>>>  			    irq_settings_is_per_cpu_devid(desc))
>>>  				return NULL;
>> 
>> The existing code is readable and obvious. This is not.
>
> Thank you for your review. The existing code is indeed clear, but it
> seems that both judgments are checking whether the percpu flags are
> consistent.

The code checks whether the descriptor is marked as per CPU devid if and
only if the caller requested it.

As the code exactly doing that, what is the point of changing it to
something incomprehensible?

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-07 10:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-06 12:50 [PATCH] genirq: Simplify the check for __irq_get_desc_lock() Jinjie Ruan
2024-05-06 17:55 ` Thomas Gleixner
2024-05-07  1:16   ` Jinjie Ruan
2024-05-07 10:20     ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).