A simple C program query ...

Given the following code inside the function ext3_write_super():
(It's there in Linux kernel 2.6.27.59)

static void ext3_write_super (struct super_block * sb)
{
        if (mutex_trylock(&sb->s_lock) != 0)
                BUG();
        sb->s_dirt = 0;
}

The conditional test at if (mutex_trylock(&sb->s_lock) != 0) is getting true? at successful lock of the sb and would execute the macro BUG()???

Given the fact that motex_trylock() is defined as below:

//File:kernel/mutex.c of Linux kernel 2.6.27.59

/***
 * mutex_trylock - try acquire the mutex, without waiting
 * @lock: the mutex to be acquired
 *
 * Try to acquire the mutex atomically. Returns 1 if the mutex
 * has been acquired successfully, and 0 on contention.
 *
 * NOTE: this function follows the spin_trylock() convention, so
 * it is negated to the down_trylock() return values! Be careful
 * about this when converting semaphore users to mutexes.
 *
 * This function must not be used in interrupt context. The
 * mutex must be released by the same task that acquired it.
 */
int __sched mutex_trylock(struct mutex *lock)
{
        return __mutex_fastpath_trylock(&lock->count,
                                        __mutex_trylock_slowpath);
}

The reason, I'm putting this pesky question is that the code is of a working ext3 filesystem (and deemed stable to a reasonable amount) however my simple logic is telling me its an issue in testing that if() condition , am I wrong in my judgement???

Please put your any comments.
Thanks in advance.

Is it possible that the lock should have already been captured and this code is just ensuring that it has the lock? In that context, the code is correct -- it's a bug if the lock wasn't obtained before invoking ext3_write_super .

If the lock is already held, then the return is 0, and thus it is safe to do the work that the function wants to do.

This might not be the case, but assuming you've posted the whole ext3_write_super function, which does not release the mutex, that'd be my guess.

Imagine that the lock is already taken, then following cases arise:
1) What if the task is not the same (which is possible and ext3_write_super() is callable concurrently (via VFS) from different kernel thread contexts which is writing on a same file system (on different volumes or same one for different writes).

In this case using sb->s_dirt in anyway is wrong.

2) Even if it's the same task which has taken lock before calling ext3_write_super() the if() block should test if (mutex_trylock(&sb->s_lock) == 0) instead of if (mutex_trylock(&sb->s_lock) != 0) .

By testing for if (mutex_trylock(&sb->s_lock) != 0) ; on new lock just taken it's calling BUG() which is designed to dump core only.