All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-06-30  5:56 Gautam Menghani
  2023-07-03  6:41 ` Michael Ellerman
  2023-07-06  7:50 ` Jordan Niethe
  0 siblings, 2 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-06-30  5:56 UTC (permalink / raw)
  To: mpe, npiggin, christophe.leroy
  Cc: Gautam Menghani, linuxppc-dev, linux-kernel

Remove an unnecessary piece of code that does an endianness conversion but
does not use the result. The following warning was reported by Clang's
static analyzer:

arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
'server' is never read [deadcode.DeadStores]
        server = be16_to_cpu(oserver);

As the result of endianness conversion is never used, delete the line
and fix the warning.

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
 arch/powerpc/sysdev/xics/ics-opal.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
index 6cfbb4fac7fb..5fe73dabab79 100644
--- a/arch/powerpc/sysdev/xics/ics-opal.c
+++ b/arch/powerpc/sysdev/xics/ics-opal.c
@@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
 		       __func__, d->irq, hw_irq, rc);
 		return -1;
 	}
-	server = be16_to_cpu(oserver);
 
 	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
 	if (wanted_server < 0) {
-- 
2.39.3


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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-06-30  5:56 [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS Gautam Menghani
@ 2023-07-03  6:41 ` Michael Ellerman
  2023-07-06  7:50 ` Jordan Niethe
  1 sibling, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2023-07-03  6:41 UTC (permalink / raw)
  To: Gautam Menghani, npiggin, christophe.leroy
  Cc: Gautam Menghani, linuxppc-dev, linux-kernel

Gautam Menghani <gautam@linux.ibm.com> writes:
> Remove an unnecessary piece of code that does an endianness conversion but
> does not use the result. The following warning was reported by Clang's
> static analyzer:
>
> arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> 'server' is never read [deadcode.DeadStores]
>         server = be16_to_cpu(oserver);
>
> As the result of endianness conversion is never used, delete the line
> and fix the warning.
>
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  arch/powerpc/sysdev/xics/ics-opal.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> index 6cfbb4fac7fb..5fe73dabab79 100644
> --- a/arch/powerpc/sysdev/xics/ics-opal.c
> +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
>  		       __func__, d->irq, hw_irq, rc);
>  		return -1;
>  	}
> -	server = be16_to_cpu(oserver);
>  
>  	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
>  	if (wanted_server < 0) {

My first question with a patch like this is always going to be "how did
the code end up like this?"

Has the code changed and this assignment became unused? If so the commit
that did that should be identified.

If the code has always been like this that's also useful to know. Or
something else happened for it to end up this way :)

The second question will be "is there actually a bug here?". ie. should
server actually be used, and the bug is not that it's a dead assignment
but rather that server is not where it should be.

cheers

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-06-30  5:56 [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS Gautam Menghani
  2023-07-03  6:41 ` Michael Ellerman
@ 2023-07-06  7:50 ` Jordan Niethe
  2023-07-11  9:33     ` Gautam Menghani
                     ` (2 more replies)
  1 sibling, 3 replies; 15+ messages in thread
From: Jordan Niethe @ 2023-07-06  7:50 UTC (permalink / raw)
  To: Gautam Menghani, mpe, npiggin, christophe.leroy
  Cc: linuxppc-dev, linux-kernel



On 30/6/23 3:56 pm, Gautam Menghani wrote:
> Remove an unnecessary piece of code that does an endianness conversion but
> does not use the result. The following warning was reported by Clang's
> static analyzer:
> 
> arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> 'server' is never read [deadcode.DeadStores]
>          server = be16_to_cpu(oserver);
> 
> As the result of endianness conversion is never used, delete the line
> and fix the warning.
> 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>

'server' was used as a parameter to opal_get_xive() in commit 
5c7c1e9444d8 ("powerpc/powernv: Add OPAL ICS backend") when it was 
introduced. 'server' was also used in an error message for the call to 
opal_get_xive().

'server' was always later set by a call to ics_opal_mangle_server() 
before being used.

Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS 
backend") used a new variable 'oserver' as the parameter to 
opal_get_xive() instead of 'server' for endian correctness. It also 
removed 'server' from the error message for the call to opal_get_xive().

It was commit bf8e0f891a32 that added the unnecessary conversion and 
never used the result.

Reviewed-by: Jordan Niethe <jniethe5@gmail.com>


> ---
>   arch/powerpc/sysdev/xics/ics-opal.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> index 6cfbb4fac7fb..5fe73dabab79 100644
> --- a/arch/powerpc/sysdev/xics/ics-opal.c
> +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
>   		       __func__, d->irq, hw_irq, rc);
>   		return -1;
>   	}
> -	server = be16_to_cpu(oserver);
>   
>   	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
>   	if (wanted_server < 0) {
> 

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-07-06  7:50 ` Jordan Niethe
@ 2023-07-11  9:33     ` Gautam Menghani
  2023-07-26  8:07     ` Gautam Menghani
  2023-07-26  9:12     ` Gautam Menghani
  2 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-11  9:33 UTC (permalink / raw)
  To: Jordan Niethe; +Cc: mpe, npiggin, christophe.leroy, linuxppc-dev, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>

Yes, thanks for the info Jordan. Just to add to this,
ics_opal_mangle_server() needs input in LE and the 'wanted_server' is
already LE as xics_get_irq_server() is returning result in LE. So the
line

`server = be16_to_cpu(oserver);'

is indeed not required.

Thanks,
Gautam

> 
> 
> > ---
> >   arch/powerpc/sysdev/xics/ics-opal.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> > index 6cfbb4fac7fb..5fe73dabab79 100644
> > --- a/arch/powerpc/sysdev/xics/ics-opal.c
> > +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> > @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
> >   		       __func__, d->irq, hw_irq, rc);
> >   		return -1;
> >   	}
> > -	server = be16_to_cpu(oserver);
> >   	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
> >   	if (wanted_server < 0) {
> > 

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-07-11  9:33     ` Gautam Menghani
  0 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-11  9:33 UTC (permalink / raw)
  To: Jordan Niethe; +Cc: linuxppc-dev, npiggin, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>

Yes, thanks for the info Jordan. Just to add to this,
ics_opal_mangle_server() needs input in LE and the 'wanted_server' is
already LE as xics_get_irq_server() is returning result in LE. So the
line

`server = be16_to_cpu(oserver);'

is indeed not required.

Thanks,
Gautam

> 
> 
> > ---
> >   arch/powerpc/sysdev/xics/ics-opal.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> > index 6cfbb4fac7fb..5fe73dabab79 100644
> > --- a/arch/powerpc/sysdev/xics/ics-opal.c
> > +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> > @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
> >   		       __func__, d->irq, hw_irq, rc);
> >   		return -1;
> >   	}
> > -	server = be16_to_cpu(oserver);
> >   	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
> >   	if (wanted_server < 0) {
> > 

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-07-06  7:50 ` Jordan Niethe
@ 2023-07-26  8:07     ` Gautam Menghani
  2023-07-26  8:07     ` Gautam Menghani
  2023-07-26  9:12     ` Gautam Menghani
  2 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-26  8:07 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Gautam Menghani, npiggin, christophe.leroy, linuxppc-dev, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> 

Hello Michael,

Do you have any more questions on this patch or is it good to go?

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-07-26  8:07     ` Gautam Menghani
  0 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-26  8:07 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Gautam Menghani, linuxppc-dev, npiggin, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> 

Hello Michael,

Do you have any more questions on this patch or is it good to go?

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-07-06  7:50 ` Jordan Niethe
@ 2023-07-26  9:12     ` Gautam Menghani
  2023-07-26  8:07     ` Gautam Menghani
  2023-07-26  9:12     ` Gautam Menghani
  2 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-26  9:12 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Gautam Menghani, npiggin, christophe.leroy, linuxppc-dev, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> 

Hello Michael, 

Do you have any more questions on this patch or is it good to go?

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-07-26  9:12     ` Gautam Menghani
  0 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-26  9:12 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Gautam Menghani, linuxppc-dev, npiggin, linux-kernel

On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> 
> 
> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> > Remove an unnecessary piece of code that does an endianness conversion but
> > does not use the result. The following warning was reported by Clang's
> > static analyzer:
> > 
> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> > 'server' is never read [deadcode.DeadStores]
> >          server = be16_to_cpu(oserver);
> > 
> > As the result of endianness conversion is never used, delete the line
> > and fix the warning.
> > 
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> 
> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> was also used in an error message for the call to opal_get_xive().
> 
> 'server' was always later set by a call to ics_opal_mangle_server() before
> being used.
> 
> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> instead of 'server' for endian correctness. It also removed 'server' from
> the error message for the call to opal_get_xive().
> 
> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> used the result.
> 
> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> 

Hello Michael, 

Do you have any more questions on this patch or is it good to go?

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-07-26  9:12     ` Gautam Menghani
@ 2023-07-29 10:54       ` Michael Ellerman
  -1 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2023-07-29 10:54 UTC (permalink / raw)
  To: Gautam Menghani
  Cc: Gautam Menghani, npiggin, christophe.leroy, linuxppc-dev, linux-kernel

Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
>> On 30/6/23 3:56 pm, Gautam Menghani wrote:
>> > Remove an unnecessary piece of code that does an endianness conversion but
>> > does not use the result. The following warning was reported by Clang's
>> > static analyzer:
>> > 
>> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
>> > 'server' is never read [deadcode.DeadStores]
>> >          server = be16_to_cpu(oserver);
>> > 
>> > As the result of endianness conversion is never used, delete the line
>> > and fix the warning.
>> > 
>> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
>> 
>> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
>> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
>> was also used in an error message for the call to opal_get_xive().
>> 
>> 'server' was always later set by a call to ics_opal_mangle_server() before
>> being used.
>> 
>> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
>> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
>> instead of 'server' for endian correctness. It also removed 'server' from
>> the error message for the call to opal_get_xive().
>> 
>> It was commit bf8e0f891a32 that added the unnecessary conversion and never
>> used the result.
>> 
>> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
>> 
>
> Hello Michael, 
>
> Do you have any more questions on this patch or is it good to go?

I was expecting you'd send a v2 with the changelog updated to include
all the extra detail Jordan added. I think it should also be tagged as
Fixes: bf8e0f891a32.

cheers

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-07-29 10:54       ` Michael Ellerman
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2023-07-29 10:54 UTC (permalink / raw)
  To: Gautam Menghani; +Cc: Gautam Menghani, linuxppc-dev, npiggin, linux-kernel

Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
>> On 30/6/23 3:56 pm, Gautam Menghani wrote:
>> > Remove an unnecessary piece of code that does an endianness conversion but
>> > does not use the result. The following warning was reported by Clang's
>> > static analyzer:
>> > 
>> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
>> > 'server' is never read [deadcode.DeadStores]
>> >          server = be16_to_cpu(oserver);
>> > 
>> > As the result of endianness conversion is never used, delete the line
>> > and fix the warning.
>> > 
>> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
>> 
>> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
>> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
>> was also used in an error message for the call to opal_get_xive().
>> 
>> 'server' was always later set by a call to ics_opal_mangle_server() before
>> being used.
>> 
>> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
>> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
>> instead of 'server' for endian correctness. It also removed 'server' from
>> the error message for the call to opal_get_xive().
>> 
>> It was commit bf8e0f891a32 that added the unnecessary conversion and never
>> used the result.
>> 
>> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
>> 
>
> Hello Michael, 
>
> Do you have any more questions on this patch or is it good to go?

I was expecting you'd send a v2 with the changelog updated to include
all the extra detail Jordan added. I think it should also be tagged as
Fixes: bf8e0f891a32.

cheers

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-07-29 10:54       ` Michael Ellerman
@ 2023-07-31 12:00         ` Gautam Menghani
  -1 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-31 12:00 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Gautam Menghani, npiggin, christophe.leroy, linuxppc-dev, linux-kernel

On Sat, Jul 29, 2023 at 08:54:26PM +1000, Michael Ellerman wrote:
> Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> > On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> >> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> >> > Remove an unnecessary piece of code that does an endianness conversion but
> >> > does not use the result. The following warning was reported by Clang's
> >> > static analyzer:
> >> > 
> >> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> >> > 'server' is never read [deadcode.DeadStores]
> >> >          server = be16_to_cpu(oserver);
> >> > 
> >> > As the result of endianness conversion is never used, delete the line
> >> > and fix the warning.
> >> > 
> >> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> >> 
> >> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> >> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> >> was also used in an error message for the call to opal_get_xive().
> >> 
> >> 'server' was always later set by a call to ics_opal_mangle_server() before
> >> being used.
> >> 
> >> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> >> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> >> instead of 'server' for endian correctness. It also removed 'server' from
> >> the error message for the call to opal_get_xive().
> >> 
> >> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> >> used the result.
> >> 
> >> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> >> 
> >
> > Hello Michael, 
> >
> > Do you have any more questions on this patch or is it good to go?
> 
> I was expecting you'd send a v2 with the changelog updated to include
> all the extra detail Jordan added. I think it should also be tagged as
> Fixes: bf8e0f891a32.
> 
> cheers

Thanks for the response. I've sent a v2 here - 
lore.kernel.org/linuxppc-dev/20230731115543.36991-1-gautam@linux.ibm.com/T/#u

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-07-31 12:00         ` Gautam Menghani
  0 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-07-31 12:00 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Gautam Menghani, linuxppc-dev, npiggin, linux-kernel

On Sat, Jul 29, 2023 at 08:54:26PM +1000, Michael Ellerman wrote:
> Gautam Menghani <Gautam.Menghani@linux.ibm.com> writes:
> > On Thu, Jul 06, 2023 at 05:50:57PM +1000, Jordan Niethe wrote:
> >> On 30/6/23 3:56 pm, Gautam Menghani wrote:
> >> > Remove an unnecessary piece of code that does an endianness conversion but
> >> > does not use the result. The following warning was reported by Clang's
> >> > static analyzer:
> >> > 
> >> > arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> >> > 'server' is never read [deadcode.DeadStores]
> >> >          server = be16_to_cpu(oserver);
> >> > 
> >> > As the result of endianness conversion is never used, delete the line
> >> > and fix the warning.
> >> > 
> >> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> >> 
> >> 'server' was used as a parameter to opal_get_xive() in commit 5c7c1e9444d8
> >> ("powerpc/powernv: Add OPAL ICS backend") when it was introduced. 'server'
> >> was also used in an error message for the call to opal_get_xive().
> >> 
> >> 'server' was always later set by a call to ics_opal_mangle_server() before
> >> being used.
> >> 
> >> Commit bf8e0f891a32 ("powerpc/powernv: Fix endian issues in OPAL ICS
> >> backend") used a new variable 'oserver' as the parameter to opal_get_xive()
> >> instead of 'server' for endian correctness. It also removed 'server' from
> >> the error message for the call to opal_get_xive().
> >> 
> >> It was commit bf8e0f891a32 that added the unnecessary conversion and never
> >> used the result.
> >> 
> >> Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
> >> 
> >
> > Hello Michael, 
> >
> > Do you have any more questions on this patch or is it good to go?
> 
> I was expecting you'd send a v2 with the changelog updated to include
> all the extra detail Jordan added. I think it should also be tagged as
> Fixes: bf8e0f891a32.
> 
> cheers

Thanks for the response. I've sent a v2 here - 
lore.kernel.org/linuxppc-dev/20230731115543.36991-1-gautam@linux.ibm.com/T/#u

Thanks,
Gautam

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

* Re: [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
  2023-06-30  6:13 Gautam Menghani
@ 2023-06-30  6:16 ` Gautam Menghani
  0 siblings, 0 replies; 15+ messages in thread
From: Gautam Menghani @ 2023-06-30  6:16 UTC (permalink / raw)
  To: mpe, npiggin, christophe.leroy; +Cc: linuxppc-dev, linux-kernel

On Fri, Jun 30, 2023 at 11:43:26AM +0530, Gautam Menghani wrote:
> Remove an unnecessary piece of code that does an endianness conversion but
> does not use the result. The following warning was reported by Clang's
> static analyzer:
> 
> arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
> 'server' is never read [deadcode.DeadStores]
>         server = be16_to_cpu(oserver);
> 
> As the result of endianness conversion is never used, delete the line
> and fix the warning.
> 
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  arch/powerpc/sysdev/xics/ics-opal.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
> index 6cfbb4fac7fb..5fe73dabab79 100644
> --- a/arch/powerpc/sysdev/xics/ics-opal.c
> +++ b/arch/powerpc/sysdev/xics/ics-opal.c
> @@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
>  		       __func__, d->irq, hw_irq, rc);
>  		return -1;
>  	}
> -	server = be16_to_cpu(oserver);
>  
>  	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
>  	if (wanted_server < 0) {
> -- 
> 2.39.3
> 

I resent this patch by mistake, please ignore this. Apologies for the
trouble.

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

* [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS
@ 2023-06-30  6:13 Gautam Menghani
  2023-06-30  6:16 ` Gautam Menghani
  0 siblings, 1 reply; 15+ messages in thread
From: Gautam Menghani @ 2023-06-30  6:13 UTC (permalink / raw)
  To: mpe, npiggin, christophe.leroy
  Cc: Gautam Menghani, linuxppc-dev, linux-kernel

Remove an unnecessary piece of code that does an endianness conversion but
does not use the result. The following warning was reported by Clang's
static analyzer:

arch/powerpc/sysdev/xics/ics-opal.c:114:2: warning: Value stored to
'server' is never read [deadcode.DeadStores]
        server = be16_to_cpu(oserver);

As the result of endianness conversion is never used, delete the line
and fix the warning.

Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
 arch/powerpc/sysdev/xics/ics-opal.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/sysdev/xics/ics-opal.c b/arch/powerpc/sysdev/xics/ics-opal.c
index 6cfbb4fac7fb..5fe73dabab79 100644
--- a/arch/powerpc/sysdev/xics/ics-opal.c
+++ b/arch/powerpc/sysdev/xics/ics-opal.c
@@ -111,7 +111,6 @@ static int ics_opal_set_affinity(struct irq_data *d,
 		       __func__, d->irq, hw_irq, rc);
 		return -1;
 	}
-	server = be16_to_cpu(oserver);
 
 	wanted_server = xics_get_irq_server(d->irq, cpumask, 1);
 	if (wanted_server < 0) {
-- 
2.39.3


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

end of thread, other threads:[~2023-07-31 12:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-30  5:56 [PATCH] arch/powerpc: Remove unnecessary endian conversion code in XICS Gautam Menghani
2023-07-03  6:41 ` Michael Ellerman
2023-07-06  7:50 ` Jordan Niethe
2023-07-11  9:33   ` Gautam Menghani
2023-07-11  9:33     ` Gautam Menghani
2023-07-26  8:07   ` Gautam Menghani
2023-07-26  8:07     ` Gautam Menghani
2023-07-26  9:12   ` Gautam Menghani
2023-07-26  9:12     ` Gautam Menghani
2023-07-29 10:54     ` Michael Ellerman
2023-07-29 10:54       ` Michael Ellerman
2023-07-31 12:00       ` Gautam Menghani
2023-07-31 12:00         ` Gautam Menghani
2023-06-30  6:13 Gautam Menghani
2023-06-30  6:16 ` Gautam Menghani

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.