diff mbox

[RFC,7/8] powerpc/xmon: Drop 'valid' from the condition inside 'dump_segments'

Message ID 1437461926-8908-7-git-send-email-khandual@linux.vnet.ibm.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Anshuman Khandual July 21, 2015, 6:58 a.m. UTC 
From: "khandual@linux.vnet.ibm.com" <khandual@linux.vnet.ibm.com>

Value of 'valid' is zero when 'esid' is zero and it does not matter
when 'esid' is non-zero. Hence the variable 'value' can be dropped
from the conditional statement. This patch does that.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/xmon/xmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Ellerman July 21, 2015, 10 a.m. UTC | #1 
On Tue, 2015-21-07 at 06:58:45 UTC, Anshuman Khandual wrote:
> From: "khandual@linux.vnet.ibm.com" <khandual@linux.vnet.ibm.com>
> 
> Value of 'valid' is zero when 'esid' is zero and it does not matter
> when 'esid' is non-zero. 

Yes it does. It tells you whether the entry is valid?

In practice maybe you only see invalid entries that are entirely zero, and so
they get skipped anyway, but that's not guaranteed.

cheers
Anshuman Khandual July 21, 2015, 11:45 a.m. UTC | #2 
On 07/21/2015 03:30 PM, Michael Ellerman wrote:
> On Tue, 2015-21-07 at 06:58:45 UTC, Anshuman Khandual wrote:
>> > From: "khandual@linux.vnet.ibm.com" <khandual@linux.vnet.ibm.com>
>> > 
>> > Value of 'valid' is zero when 'esid' is zero and it does not matter
>> > when 'esid' is non-zero. 
> Yes it does. It tells you whether the entry is valid?

Yeah but it does not change the outcome of the if condition check
here. Non-zero esid will make the condition test pass irrespective
of the value of 'valid'. Yes, valid will be checked inside the code
block to print details, the point was value of valid does not make
any difference to the 'if' condition check in the first place.
Unless I am getting tricked here some how :)

> 
> In practice maybe you only see invalid entries that are entirely zero, and so
> they get skipped anyway, but that's not guaranteed.
Michael Ellerman July 22, 2015, 4:52 a.m. UTC | #3 
On Tue, 2015-07-21 at 17:15 +0530, Anshuman Khandual wrote:
> On 07/21/2015 03:30 PM, Michael Ellerman wrote:
> > On Tue, 2015-21-07 at 06:58:45 UTC, Anshuman Khandual wrote:
> >> > From: "khandual@linux.vnet.ibm.com" <khandual@linux.vnet.ibm.com>
> >> > 
> >> > Value of 'valid' is zero when 'esid' is zero and it does not matter
> >> > when 'esid' is non-zero. 
> > Yes it does. It tells you whether the entry is valid?
> 
> Yeah but it does not change the outcome of the if condition check
> here. Non-zero esid will make the condition test pass irrespective
> of the value of 'valid'. Yes, valid will be checked inside the code
> block to print details, the point was value of valid does not make
> any difference to the 'if' condition check in the first place.
> Unless I am getting tricked here some how :)

No you're right, I was confused by the bitwise or.

Please make it:  if (esid || vsid)

And drop valid entirely, just do the check in the if condition.

And fix the change log:

  Hence the variable 'value'
                      ^
		      valid


cheers
diff mbox

Patch

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index e599259..1798e21 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2740,7 +2740,7 @@  void dump_segments(void)
 		asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
 		asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
 		valid = (esid & SLB_ESID_V);
-		if (valid | esid | vsid) {
+		if (esid | vsid) {
 			printf("%02d %016lx %016lx", i, esid, vsid);
 			if (valid) {
 				llp = vsid & SLB_VSID_LLP;