RE: [Xen-devel] [PATCH 1/1] Xen ARINC 653 Scheduler (updated to add support for CPU pools)

["Kathy Hadley" <Kathy.Hadley@xxxxxxxxxxxxxxx>]

George,
  I actually tried the xmalloc() method first.  I found that when the 
.adjust_global function was called, the address of the "ops" data structure 
passed to that function was different from the address of the "ops" data 
structure when the .init function was called.  I wanted to use .adjust_global 
to modify the data structure that was created when the .init function was 
called, but I could not figure out a way to get the address of the second data 
structure.  Suggestions?

  I can make the modifications you suggest for the other items.  Thanks for the 
comments.

  Regards,
Kathy Hadley
DornerWorks, Ltd.

> -----Original Message-----
> From: dunlapg@xxxxxxxxx [mailto:dunlapg@xxxxxxxxx] On Behalf Of George
> Dunlap
> Sent: Wednesday, June 16, 2010 11:50 AM
> To: Kathy Hadley
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; Keir Fraser
> Subject: Re: [Xen-devel] [PATCH 1/1] Xen ARINC 653 Scheduler (updated
> to add support for CPU pools)
> 
> On Wed, Jun 16, 2010 at 4:04 PM, Kathy Hadley
> <Kathy.Hadley@xxxxxxxxxxxxxxx> wrote:
> >
> +/*********************************************************************
> *****
> > + * Global
> data                                                            *
> > +
> >
> ***********************************************************************
> ***/
> > +static arinc653_sched_private_t arinc653_schedule;
> [snip]
> > +    /* Allocate memory for ARINC 653 scheduler data structure */
> > +    prv = &arinc653_schedule;
> 
> You didn't actually allocate memory, you just used the static
> structure.  The point of cpupools is to allow multiple instances of a
> scheduler to coexist -- if people create two pools, both using the
> ARINC scheduler, there will be problems with this.  Is there any
> reason not to actually call xmalloc() (as is done in
> sched_credit.c:csched_init())?  (Perhaps this is a missed FIXME or a
> merge-o?)
> 
> Some of the notices in headers seems a little excessive; if
> sched_arinc653.c credits Dornerworks, surely people can infer who
> added the control structure in xen/include/public/sysctl.h, and added
> a link to it in scheduler.c?
> 
> Not NACK-worthy, but: In struct arin..._sched_private_s, the element
> "arinc653_schedule" should probably be named something a bit more
> descriptive.  Similarly, having arinc653 in ..._major_frame seems a
> bit redundant, and inconsistent with naming for the other elements.
> 
> Looks fine to me otherwise.  (I haven't reviewed the algorithm itself.)
> 
>  -George

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