On Mon, 2006-06-05 at 09:00 +0400, Evgeniy Dushistov wrote:
> On Sun, Jun 04, 2006 at 04:49:56PM -0500, Dave Kleikamp wrote:
> > On Sun, 2006-06-04 at 19:44 +0400, Evgeniy Dushistov wrote:
> > > I should add that this happened during boot, when root jfs
> > > file system become from ro->rw
> > >
> > > I look at code, and see that
> > > 1)locks wasn't release in the opposite order in which
> > > they were taken
> >
> > Why does this matter?
> >
> Like "3" it make code more understandable,
> reader of code don't have to think why
> code looks like:
> lock A, lock B unlock A, unlock B
> while a normal practice in kernel:
> lock A, lock B unlock B unlock A
> the goal of change just simplicity and clearness,
> and nothing more.
Well, when the locks are released at one place, I don't buy that it make
the code any easier to understand. However, I understand that it makes
it easier for the validator to do its job, so I'm happy with all three
parts of the patch. I'll push it to the -mm tree.
>
> > I think the warning needs to be fixed by introducing mutex_lock_nested
> > in some places. I'll take a look at it.
> >
> To avoid incomprehension, previous patch just make code more
> understandable and unlock mutex on "error path", it doesn't fix
> this warning and I have no idea why it happend:
I understand the warning. jfs is taking the same mutex (commit_mutex)
on two inodes, which has the potential to for two threads to deadlock.
I believe the order the locks are taken is safe (always taking the
parent first). In fact, I think any operations involving more than one
inode are already protected by i_mutex on the parents. I'm sure I can
fix it with mutex_lock_nested.
> ====================================
> [ BUG: possible deadlock detected! ]
> ------------------------------------
> mount/5587 is trying to acquire lock:
> (&jfs_ip->commit_mutex){--..}, at: [<c02f7096>] mutex_lock+0x12/0x15
>
> but task is already holding lock:
> (&jfs_ip->commit_mutex){--..}, at: [<c02f7096>] mutex_lock+0x12/0x15
>
> which could potentially lead to deadlocks!
>
> other info that might help us debug this:
> 2 locks held by mount/5587:
> #0: (&inode->i_mutex){--..}, at: [<c02f7096>] mutex_lock+0x12/0x15
> #1: (&jfs_ip->commit_mutex){--..}, at: [<c02f7096>] mutex_lock+0x12/0x15
Thanks,
Shaggy
--
David Kleikamp
IBM Linux Technology Center
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
