Re: Sparc64, cpumask_t and struct arguments (was: [PATCH] Introduce nodemask_t ADT)

[Paul Jackson <pj@sgi.com>]

> >  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)
						cpu_clear(i, mask);

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@sgi.com> 1.650.933.1373