Re: [Xen-devel] xentrace_setmask badly broken?

[Mark Williamson <mark.williamson@xxxxxxxxxxxx>]

> I was just looking at tools/xenmon/setmask.c which gets installed as
> /usr/sbin/xentrace_setmask.
>
> Am I confused or is this badly broken (both 3.1.x and 3.2)?  Two problems:

It's not actually functionally broken but it does have some issues.

> 1) Command line arguments to set the xentrace mask are completely ignored,

It looks like the tool just sets things up the way Xenmon needs them...  This 
is fine but I wonder if it couldn't just be rolled into the xenbaked?  If 
it's a necessary special-purpose tool for xenmon, it should probably be 
stashed away off the user's path somewhere.

> and 2) The mask that IS always created and set is a bitwise-or of eight
> constants that are integers, not bitmasks, resulting in a mask which should
> be useless, except that the result -- either by accident or devious coding
> style -- comes out to be a reasonable value for a mask.

The specific event within a (sub)class is identified by the low 12 bits of the 
event ID.  These are just integers and are not suitable to be ORed.

However...

The Class and Subclass of the event are chosen from a set of values which are 
safe to be ORed together, and these are in the upper bits of the event ID.  
The result of this is that the OR in setmask.c actually will do the right 
thing, since the mask is really only concerned with class and subclass.  So 
the code does actually do the right thing in terms of masking the correct 
(sub)classes.

The test in line 376 of trace.c will sometimes not return when you'd really 
want it to, although I doubt that has much impact it'd still be nice to rule 
that out.

> Looks like this has been broken since the beginning of (its) time so no
> rush to fix it, but if someone would be so kind as to confirm I am not
> seeing things, I will work up a patch.

The code in setmask.c should probably be changed to be clearer: it works as-is 
but the intent would be much clearer if Class identifier were ORed rather 
than individual IDs.  If my understanding is correct then this should work.

Whilst you're about it ;-) I suggest Xen ought to check the trace mask (i.e. 
low twelve bits should be zero).  Maybe just reject an invalid mask with some 
appropriate error return?

Happy New Year all!

Cheers,
Mark

-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel