[Tinyos-devel] Msp430RefVoltArbiterImplP.nc switchOff loop

[Maxime Muller - s h o c k f i s h /]

Hi Jan,

Thanks for this quick fix, it's working fine on my side.
i agree about that possible race.

Regards,
maxime

> Hi Maxime,
> yes, this is a bug. I think your solution is good, but in some edge
> cases can lead to a race condition: specifically, if in
> ClientResource.release() "owner = NO_OWNER" is executed, and then
> another client's call to ClientResource.immediateRequest() happens
> before switchOff() is executed. Then the "owner" variable is
> overridden, switchOff() will do nothing and the generator remains on
> although it should be off.
>
> Instead I changed the "owner" variable to become a "syncOwner"
> variable, which I only modify in sync (task) context and have the
> async path "parallel" to it (it's hard to explain take a look at the
> diff) - I've committed it to CVS and put my test app under
> tinyos-2.x/apps/tests/msp430/Adc12.
>
> Jan
>
>
> On 4/5/07, Philip Levis <pal at cs.stanford.edu> wrote:
>
>> On Apr 4, 2007, at 8:37 AM, Maxime Muller - s h o c k f i s h / wrote:
>>
>> > Hi all,
>> >
>> > On tos 2.x, file $TOSDIR/chips/msp430/adc12/
>> > Msp430RefVoltArbiterImplP.nc
>> > during event void AdcResource.granted[uint8_t client]()
>> > If your sref is not REFERENCE_VREFplus_AVss ||
>> > REFERENCE_VREFplus_VREFnegterm
>> > then you will never call Refvolt_1/2_5V.start()
>> >
>> > Now while releasing the resource, imho, we should do the same check
>> > else we end up with a post switchOff() loop
>> > as call RefVolt_1_5V.stop() will fail.
>> >
>> > async command error_t ClientResource.release[uint8_t client]()
>> >  {
>> >      const msp430adc12_channel_config_t* settings  = call
>> > Config.getConfiguration[client]();
>> >      atomic {
>> >      if (owner == client){                  owner = NO_OWNER;
>> >          if (settings->sref == REFERENCE_VREFplus_AVss ||
>> >          settings->sref == REFERENCE_VREFplus_VREFnegterm)
>> >          post switchOff();
>> >      }
>> >      }
>> >    return call AdcResource.release[client]();
>> >  }
>> > seems better or did i miss something?
>>
>> We discussed this in the core WG meeting today. Jan (who's
>> responsible for the MSP430 ADC) is going to take a look. The short
>> story is that you've definitely found a bug, but your solution has a
>> race condition in it. Our hope is to fix this for the 2.0.1 release;
>> Jan will hopefully be getting in touch with you about it.
>>
>> Phil
>>