If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
would the following code be SMP safe?
hold_lock = mutex_trylock()
...
if (hold_lock)
mutex_unlock()
Thanks,
Larry
Larry Finger wrote:
> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> would the following code be SMP safe?
>
> hold_lock = mutex_trylock()
>
> ..
>
> if (hold_lock)
> mutex_unlock()
Not if another task could be acquiring that lock at the same time, which
is probably the case, otherwise you wouldn't need the mutex. In other
words, if you're going to do this, you might as well toss the mutex
entirely as it's about the same effect..
--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/
Robert Hancock wrote:
> Larry Finger wrote:
>> If a particular routine needs to lock a mutex, but it may be entered
>> with that mutex already locked,
>> would the following code be SMP safe?
>>
>> hold_lock = mutex_trylock()
>>
>> ..
>>
>> if (hold_lock)
>> mutex_unlock()
>
> Not if another task could be acquiring that lock at the same time, which
> is probably the case, otherwise you wouldn't need the mutex. In other
> words, if you're going to do this, you might as well toss the mutex
> entirely as it's about the same effect..
>
Thanks for the help. Someday, I hope to understand this stuff.
Larry
El Tue, Nov 27, 2007 at 10:37:00PM -0600 Larry Finger ha dit:
> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> would the following code be SMP safe?
>
> hold_lock = mutex_trylock()
>
> ...
>
> if (hold_lock)
> mutex_unlock()
this is wont work, a mutex must not be released from another
context than the one that acquired it.
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
Don't walk behind me, I may not lead
Don't walk in front of me, I may not follow
Just walk beside me and be my friend
(Albert Camus)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
Matthias Kaehlcke wrote:
> El Tue, Nov 27, 2007 at 10:37:00PM -0600 Larry Finger ha dit:
>
>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
>> would the following code be SMP safe?
>>
>> hold_lock = mutex_trylock()
>>
>> ...
>>
>> if (hold_lock)
>> mutex_unlock()
>
> this is wont work, a mutex must not be released from another
> context than the one that acquired it.
I thought that mutex_trylock() returned 1 if it got the lock and 0 if not. If that is true, wouldn't
the if statement only unlock if the lock was obtained in this routine?
El Wed, Nov 28, 2007 at 08:46:51AM -0600 Larry Finger ha dit:
> Matthias Kaehlcke wrote:
> > El Tue, Nov 27, 2007 at 10:37:00PM -0600 Larry Finger ha dit:
> >
> >> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> >> would the following code be SMP safe?
> >>
> >> hold_lock = mutex_trylock()
> >>
> >> ...
> >>
> >> if (hold_lock)
> >> mutex_unlock()
> >
> > this is wont work, a mutex must not be released from another
> > context than the one that acquired it.
>
> I thought that mutex_trylock() returned 1 if it got the lock and 0 if not. If that is true, wouldn't
> the if statement only unlock if the lock was obtained in this routine?
you're right, sorry i read to fast and interpreted that you want to
release the mutex in any case
--
Matthias Kaehlcke
Linux Application Developer
Barcelona
You must have a plan. If you don't have a plan,
you'll become part of somebody else's plan
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
Larry Finger <[email protected]> writes:
> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> would the following code be SMP safe?
>
> hold_lock = mutex_trylock()
>
> ...
>
> if (hold_lock)
> mutex_unlock()
When two CPUs may enter the critical region at the same time, what is
the point of the mutex? Also, the first CPU may unlock the mutex while
the second one is still inside the critical region.
Andreas.
--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstra?e 5, 90409 N?rnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Andreas Schwab wrote:
> Larry Finger <[email protected]> writes:
>
>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
>> would the following code be SMP safe?
>>
>> hold_lock = mutex_trylock()
>>
>> ...
>>
>> if (hold_lock)
>> mutex_unlock()
>
> When two CPUs may enter the critical region at the same time, what is
> the point of the mutex? Also, the first CPU may unlock the mutex while
> the second one is still inside the critical region.
Thank you for that answer. I think that I'm finally beginning to understand.
Larry
Larry Finger wrote, On 11/28/2007 04:41 PM:
> Andreas Schwab wrote:
>> Larry Finger <[email protected]> writes:
>>
>>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
>>> would the following code be SMP safe?
>>>
>>> hold_lock = mutex_trylock()
>>>
>>> ...
>>>
>>> if (hold_lock)
>>> mutex_unlock()
>> When two CPUs may enter the critical region at the same time, what is
>> the point of the mutex? Also, the first CPU may unlock the mutex while
>> the second one is still inside the critical region.
>
> Thank you for that answer. I think that I'm finally beginning to understand.
Probably it would be faster without these "...", which look like
no man's land...
hold_lock = mutex_trylock()
if (hold_lock) {
/* SMP safe */
...
mutex_unlock()
} else {
/* SMP unsafe */
...
/* maybe try again after some break or check */
}
Regards,
Jarek P.
Jarek Poplawski wrote, On 11/28/2007 11:45 PM:
> Larry Finger wrote, On 11/28/2007 04:41 PM:
>
>> Andreas Schwab wrote:
>>> Larry Finger <[email protected]> writes:
>>>
>>>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
>>>> would the following code be SMP safe?
>>>>
>>>> hold_lock = mutex_trylock()
>>>>
>>>> ...
>>>>
>>>> if (hold_lock)
>>>> mutex_unlock()
>>> When two CPUs may enter the critical region at the same time, what is
>>> the point of the mutex? Also, the first CPU may unlock the mutex while
>>> the second one is still inside the critical region.
>> Thank you for that answer. I think that I'm finally beginning to understand.
>
> Probably it would be faster without these "...", which look like
> no man's land...
>
> hold_lock = mutex_trylock()
> if (hold_lock) {
> /* SMP safe */
> ...
> mutex_unlock()
> } else {
> /* SMP unsafe */
> ...
> /* maybe try again after some break or check */
OOPS! Of course, since it can be called with this lock held,
any break is not enough: we can only check if there is a
possibility that another thread is holding the lock.
> }
>
> Regards,
> Jarek P.
Jarek Poplawski wrote, On 11/28/2007 11:56 PM:
> Jarek Poplawski wrote, On 11/28/2007 11:45 PM:
>
>> Larry Finger wrote, On 11/28/2007 04:41 PM:
>>
>>> Andreas Schwab wrote:
>>>> Larry Finger <[email protected]> writes:
>>>>
>>>>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
>>>>> would the following code be SMP safe?
>>>>>
>>>>> hold_lock = mutex_trylock()
>>>>>
>>>>> ...
>>>>>
>>>>> if (hold_lock)
>>>>> mutex_unlock()
>>>> When two CPUs may enter the critical region at the same time, what is
>>>> the point of the mutex? Also, the first CPU may unlock the mutex while
>>>> the second one is still inside the critical region.
>>> Thank you for that answer. I think that I'm finally beginning to understand.
>> Probably it would be faster without these "...", which look like
>> no man's land...
>>
>> hold_lock = mutex_trylock()
>> if (hold_lock) {
>> /* SMP safe */
>> ...
>> mutex_unlock()
>> } else {
>> /* SMP unsafe */
...But, not for sure! If our caller holds the lock and we can
check this...
>> ...
>> /* maybe try again after some break or check */
>
>
> OOPS! Of course, since it can be called with this lock held,
> any break is not enough: we can only check if there is a
> possibility that another thread is holding the lock.
>
>> }
>>
>> Regards,
>> Jarek P.
>
>
On Wed, 28 Nov 2007 23:45:13 +0100
Jarek Poplawski <[email protected]> wrote:
> Larry Finger wrote, On 11/28/2007 04:41 PM:
>
> > Andreas Schwab wrote:
> >> Larry Finger <[email protected]> writes:
> >>
> >>> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> >>> would the following code be SMP safe?
> >>>
> >>> hold_lock = mutex_trylock()
> >>>
> >>> ...
> >>>
> >>> if (hold_lock)
> >>> mutex_unlock()
> >> When two CPUs may enter the critical region at the same time, what is
> >> the point of the mutex? Also, the first CPU may unlock the mutex while
> >> the second one is still inside the critical region.
> >
> > Thank you for that answer. I think that I'm finally beginning to understand.
>
> Probably it would be faster without these "...", which look like
> no man's land...
>
> hold_lock = mutex_trylock()
> if (hold_lock) {
> /* SMP safe */
> ...
> mutex_unlock()
> } else {
> /* SMP unsafe */
> ...
> /* maybe try again after some break or check */
> }
>
> Regards,
> Jarek P.
WTF are you teaching a lesson on how NOT to do locking?
Any code which has this kind of convoluted dependency on conditional
locking is fundamentally broken.
--
Stephen Hemminger <[email protected]>
> Thanks for the help. Someday, I hope to understand this stuff.
>
> Larry
Any code either deals with an object or it doesn't. If it doesn't deal with
that object, it should not be acquiring locks on that object. If it does
deal with that object, it must know the internal details of that object,
including when and whether locks are held, or it cannot deal with that
object sanely.
So your question starts out broken, it says, "I need to lock an object, but
I have no clue what's going on with that very same object." If you don't
know what's going on with the object, you don't know enough about the object
to lock it. If you do, you should know whether you hold the lock or not.
Either architect so this function doesn't deal with that object and so
doesn't need to lock it or architect it so that this function knows what's
going on with that object and so knows whether it holds the lock or not.
If you don't follow this rule, a lot of things can go horribly wrong. The
two biggest issues are:
1) You don't know the semantic effect of locking and unlocking the mutex. So
any code placed before the mutex is acquired or after its released may not
do what's expected. For example, you cannot unlock the mutex and yield,
because you might not actually wind up unlocking the mutex.
2) A function that acquires a lock normally expects the object it locks to
be in a consistent state when it acquires the lock. However, since your code
may or may not acquire the mutex, it is not assured that its lock gets the
object in a consistent state. Requiring the caller to know this and call the
function with the object in a consistent state creates brokenness of varying
kinds. (If the object may change, why not just release the lock before
calling? If the object may not change, why is the sub-function releasing the
lock?)
DS
On Wed, Nov 28, 2007 at 03:33:12PM -0800, Stephen Hemminger wrote:
...
> WTF are you teaching a lesson on how NOT to do locking?
>
> Any code which has this kind of convoluted dependency on conditional
> locking is fundamentally broken.
>
As a matter of fact I've been thinking, about one more Re: to myself
to point this all is a good example how problematic such solution
would be, but I've decided it's rather apparent. IMHO learning needs
bad examples too - to better understand why they should be avoided.
On the other hand, I've seen quite a lot of fundamentally right, but
practically broken code, so I'm not sure what's better. And, btw., I
guess this 'fundamentally broken' type of locking could be found in
the kernel too, but I'd prefer not too look after this now.
Thanks,
Jarek P.
On 29-11-2007 03:34, David Schwartz wrote:
>> Thanks for the help. Someday, I hope to understand this stuff.
>>
>> Larry
>
> Any code either deals with an object or it doesn't. If it doesn't deal with
> that object, it should not be acquiring locks on that object. If it does
> deal with that object, it must know the internal details of that object,
> including when and whether locks are held, or it cannot deal with that
> object sanely.
...
Maybe it'll unnecessarily complicate the thing, but since you repeat
the need to know the object - sometimes the locking is done to
synchronize something in time only, so to assure only one action is
done at a time or a few actions are done in proper order, or/and
shouldn't be broken in the meantime by other actions (so, no need
to deal with any common data).
But, of course, we can say an action could be a kind of object too.
Regards,
Jarek P.
Larry Finger wrote:
> If a particular routine needs to lock a mutex, but it may be entered with that mutex already locked,
> would the following code be SMP safe?
>
> hold_lock = mutex_trylock()
The common way to deal with this is first to restructure your function
into two. One always acquires the lock, and the other (often written
with a "__" prefix) never acquires it. The never-acquire code does the
actual work, and the always-acquire function calls it.
You then refactor the callers so that you don't have any code paths on
which you can't predict whether or not the lock will be held.
<b