Re: [Xen-devel] [PATCH RFC v2 1/4] xen: add real time scheduler rt

[Meng Xu <xumengpanda@xxxxxxxxx>]

Hi Andrew,

âThank you so much for your advice! They are very useful! :-)

I have one simple question about your comments:Â

â

> +static void *
> +rt_alloc_domdata(const struct scheduler *ops, struct domain *dom)
> +{
> + Â Âunsigned long flags;

> + Â Âstruct rt_dom *sdom;

> + Â Âstruct rt_private * prv = RT_PRIV(ops);
> +
> + Â Âprinttime();
> + Â Âprintk("dom=%d\n", dom->domain_id);
> +
> + Â Âsdom = xzalloc(struct rt_dom);
> + Â Âif ( sdom == NULL )
> + Â Â{
> + Â Â Â Âprintk("%s, xzalloc failed\n", __func__);
> + Â Â Â Âreturn NULL;
> + Â Â}
> +
> + Â ÂINIT_LIST_HEAD(&sdom->vcpu);
> + Â ÂINIT_LIST_HEAD(&sdom->sdom_elem);
> + Â Âsdom->dom = dom;
> +
> + Â Â/* spinlock here to insert the dom */
> + Â Âspin_lock_irqsave(&prv->lock, flags);
> + Â Âlist_add_tail(&sdom->sdom_elem, &(prv->sdom));
> + Â Âspin_unlock_irqrestore(&prv->lock, flags);
> +
> + Â Âreturn (void *)sdom;

Bogus cast.

âI think we have to cast it to void * âbecause the definition of this function asks the return type to be void *. In addition, the credit2 scheduler also did the same cast in Âthis _alloc_domdata function. So I guess this should be fine?

In C, all pointers are implicitly castable to/from void*. This is not the case in C++. (which suggests to me that George learnt C++ before C, or is at least more familiar with C++?)

The act of using type punning means that you are trying to do something which the C type system doesn't like. This in turn requires more thought from people reading the code to work out whether it is actually safe or not. As a result, we strive for as few type overrides as possible.

Please don't fall into the trap of assuming the credit2 code, or indeed any of the other code in Xen, is perfect. None of it is, although most of it is good.

Â

âI can change the return (void *)sdom to return sdom.Â

âBut I'm not sure what else should I modify?Â

In sched-if.h, Âthis function is â"void * Â Â Â (*alloc_domdata) Â(const struct scheduler *, struct domain *);" I think I should not change this declaration, otherwise, the modification will affect the compilation of other schedulers.

ââ

âThank you so much!

Best,

Mengâ

--

-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel