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

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 16/06/2010 17:00, "Kathy Hadley" <Kathy.Hadley@dornerworks.com> wrote:

> 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?

You should modify the structure you are passed -- that is ops and your
private structure as pointed at via ops->sched_data. The latter should
always point at a private structure you previously initialised via your
.init hook.

 -- Keir

>   I can make the modifications you suggest for the other items.  Thanks for
> the comments.
> 
>   Regards,
> Kathy Hadley
> DornerWorks, Ltd.
> 
>> -----Original Message-----
>> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George
>> Dunlap
>> Sent: Wednesday, June 16, 2010 11:50 AM
>> To: Kathy Hadley
>> Cc: xen-devel@lists.xensource.com; 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@dornerworks.com> 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