lkml.org 
[lkml]   [2018]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] Revert "workqueue: re-add lockdep dependencies for flushing"
From
Date
On 10/22/18 2:04 PM, Johannes Berg wrote:
> On Mon, 2018-10-22 at 13:54 -0700, Bart Van Assche wrote:
>> The code in the column with label "CPU0" is code called by do_blockdev_direct_IO().
>> From the body of that function:
>>
>> /* will be released by direct_io_worker */
>> inode_lock(inode);
>
> I don't think this is related. If this comment is true (and I have no
> reason to believe it's not), then the inode lock is - by nature of
> allowing lock/unlock to happen in different processes - not something
> lockdep can track to start with.
>
> [ ... ]
>> You do realize that this workqueue tracking stuff has been around for
> a few years (and got removed again in refactoring, etc.) and has found > countless bugs?

This is something I had not realized when I posted the patch at the
start of this e-mail thread. Thanks for having mentioned this.

But I doubt that the inode lock has been annotated incorrectly. From the
kernel source code:

static inline void inode_lock(struct inode *inode)
{
down_write(&inode->i_rwsem);
}

[ ... ]

void __sched down_write(struct rw_semaphore *sem)
{
might_sleep();
rwsem_acquire(&sem->dep_map, 0, 0, _RET_IP_);

LOCK_CONTENDED(sem, __down_write_trylock, __down_write);
rwsem_set_owner(sem);
}

It seems to me that the inode lock has been annotated correctly as an
rwsem. It's not clear to me however why lockdep complains about a
deadlock for the direct I/O code. I hope someone has the time to go to
the bottom of this.

Bart.

\
 
 \ /
  Last update: 2018-10-23 03:17    [W:0.077 / U:2.964 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site