Re: Sparc64, cpumask_t and struct arguments (was: [PATCH] Introducenodemask_t ADT)
From: Paul Jackson
Date: Fri Mar 26 2004 - 18:23:31 EST
> > 5) The cpu_clear(i, mask) on about line 534 of smp.c confuses me, as it
> > seems to be changing a local copy of 'mask' that no one will examine
> > later. What purpose does it serve? See this line annotated with
> > "No affect??" in the changes, below.
> No, mask is the top level mask value, we assign it to local variable
> in order to do this or that operation on it. Once we've really send
> the XCALL message to the processor, we clear it in 'mask' only then.
Yes - I see where the input argument 'mask' is assigned to 'work_mask',
and then 'work_mask' is manipulated during the loops.
But inside the last loop of cheetah_xcall_deliver(), after 'mask' has
been read the last time to initialize 'work_mask', it is repeatedly
updated with the lines:
if ((dispatch_stat & check_mask) == 0)
Why, why? Can't these two lines just be _removed_?
What earthly purpose does that 'cpu_clear(i, mask)' have?
That copy of 'mask' will never be used again, at least not that I can see.
There's even a comment a few lines above, seemingly related to this:
/* Clear out the mask bits for cpus which did not
* NACK us.
But that doesn't address the "why".
And then, if these two lines can be removed for lack of affect, the six
lines just before, that setup 'check_mask', can go too, also for lack of
any remaining affect.
That would be eight lines of dead code, with three lines of commentary.
Sorry - I must be missing something somewhere.
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <pj@xxxxxxx> 1.650.933.1373
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/