Re: Inapropriate inlining in ci/include/cluster/synch.h

John Hughes <[email protected]> Mon, 02 Feb 2009 18:10:51 +0100
Newsgroups gmane.linux.cluster.ssic.devel
Message-ID <[email protected]>
John Hughes wrote:
> John Hughes wrote:
>> We have:
>>
>> static inline void _ssi_wait_event(EVENT_T *eventp, int intr, int 
>> *was_intrp)
>> {
>>         EVENT_WAIT_T evwait;
>>         sigset_t old_blocked;
>>         task_t *tsk = current;
>>         unsigned long flags;
>>         int interrupted = 0;
>>
>> That's rather a lot to inline.  The 3 calls in icssvr_daemon take up 
>> around 84 bytes of stack.
>>   
> A patch.
Note that that patch had a horrible bug:

static inline int _ssi_try_lock_lock(LOCK_T *lockp _SSI_LOCK_WHERE)
 {
-	int locked;
-
 	_SSI_LOCK_ASSERT_INITED(lockp);
 	_SSI_LOCK_ASSERT(lockp->sl_owner != current);
 	_SSI_LOCK_ASSERT(!in_interrupt());
-	locked = !down_trylock(&lockp->sl_semaphore);
-	if (locked)
+	if (down_trylock(&lockp->sl_semaphore)) {
 		lockp->sl_owner = current;
+		return 1;
+	}
 
-	return locked;
+	return 0;
 }

down_trylock returns 0 for success!

Aaargh!



------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword