Arjan van de Ven:
> what do you do if the trylock fails?
Just do not read the status variable now but modify the timer to run later.
> to be honest, the scenario describe really smells of broken locking, in
> fact it really sounds like it wants to use spinlocks instead
No, I don't think it is broken.
Spinlocks can be used, but I don't see them being obviously better in all cases.
If access takes a long time, it is better to sleep during it.
And if you sleep, you might just end up creating a new mutex implementation with a spinlock.
Alan Cox:
> For polling and timer based code its often simpler to do
>
> del_timer_sync(&my_timer);
> FrobStuff
> add_timer(&my_timer);
>
> especially if "FrobStuff" is likely to change when you next need to poll.
In the scenario I presented, the timer modifies itself to run later.
Therefore, simply calling del_timer_sync is not enough but you have to set an atomic variable to prevent the timer from adding itself again.
Again, you end up creating a new mutex implementation, which is not good.
__________________________________
Yahoo! Clever - Der einfachste Weg, Fragen zu stellen und Wissenswertes mit Anderen zu teilen. http://www.yahoo.de/clever
[re-adding CCs, please do not drop these]
On Tue, 2007-09-11 at 09:20 -0700, Matti Linnanvuori wrote:
> Arjan van de Ven:
> > what do you do if the trylock fails?
>
> Just do not read the status variable now but modify the timer to run later.
>
> > to be honest, the scenario describe really smells of broken locking, in
> > fact it really sounds like it wants to use spinlocks instead
>
> No, I don't think it is broken.
Yes it is.
> Spinlocks can be used, but I don't see them being obviously better in all cases.
> If access takes a long time, it is better to sleep during it.
> And if you sleep, you might just end up creating a new mutex
> implementation with a spinlock.
If you have to wait a long time in an atomic context you've done
something wrong. If you're only reading it from an atomic context you
might consider storing a copy that can be quickly updated and protect
that using a spinlock.
do_update ()
{
mutex_lock(&my_device_mutex);
my_device_frob_state(&my_state); /* <-- this takes a _long_ while */
spin_lock_irq(&my_shadow_state_lock);
my_shadow_state = state; /* <-- this is a quick memcopy */
spin_unlock_irq(&my_shadow_state_lock);
mutex_unlock(&my_device_mutex);
}
do_read()
{
spin_lock_irq(&my_shadow_state);
do_something_with_shadow_state(&mt_shadow_state);
spin_unlock_irq(&my_shadow_state);
return foo;
}
> Alan Cox:
> > For polling and timer based code its often simpler to do
> >
> > del_timer_sync(&my_timer);
> > FrobStuff
> > add_timer(&my_timer);
> >
> > especially if "FrobStuff" is likely to change when you next need to poll.
>
> In the scenario I presented, the timer modifies itself to run later.
> Therefore, simply calling del_timer_sync is not enough but you have to
> set an atomic variable to prevent the timer from adding itself again.
Not being too familiar with the timer stuff, it smells wrong what you
say.
As for the whole polling method, consider what Alan said, don't do it if
you don't need to. You'll annoy people at no end. Try to push state
changes where possible.
On Tue, 11 Sep 2007 09:20:23 -0700 (PDT)
Matti Linnanvuori <[email protected]> wrote:
> Arjan van de Ven:
> > what do you do if the trylock fails?
>
> Just do not read the status variable now but modify the timer to run
> later.
>
> > to be honest, the scenario describe really smells of broken
> > locking, in fact it really sounds like it wants to use spinlocks
> > instead
>
> No, I don't think it is broken.
> Spinlocks can be used, but I don't see them being obviously better in
> all cases. If access takes a long time, it is better to sleep during
> it. And if you sleep, you might just end up creating a new mutex
> implementation with a spinlock.
at this point the discussion has gone so theoretical that I think it's
better to go with a real example. What actual source code do you think
is a legit case for this?
I still think that whatever case you have in mind is better served with
something else, but until we see the actual complete drier we're both
talking air.