WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] xentrace_setmask badly broken?

To: xen-devel@xxxxxxxxxxxxxxxxxxx, "dan.magenheimer@xxxxxxxxxx" <dan.magenheimer@xxxxxxxxxx>
Subject: Re: [Xen-devel] xentrace_setmask badly broken?
From: Mark Williamson <mark.williamson@xxxxxxxxxxxx>
Date: Thu, 3 Jan 2008 01:51:04 +0000
Cc: "rdg@xxxxxx" <rdg@xxxxxx>
Delivery-date: Wed, 02 Jan 2008 17:51:33 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20080102151826296.00000000500@djm-pc>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20080102151826296.00000000500@djm-pc>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.9.6 (enterprise 0.20070907.709405)
> 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

<Prev in Thread] Current Thread [Next in Thread>