On 10/6/20 5:50 AM, Pavel Reichl wrote: >> Also, we're not really releasing the lock itself here, right? We're >> merely updating lockdep's bookkeepping so that the worker can make >> itself look like the lock owner (to lockdep, anyway). > Hmm...I'm afraid I don't follow - yes we are doing this to satisfy lockdep's bookkeeping, > however we actually do this by releasing the lock in one kernel thread and acquiring it in another. it's the difference between actually releasing the lock itself, and telling lockdep that we're releasing the "ownership" of the lock for tracking purposes; I agree that "rwsem_release" is a bit confusingly named. > >> Does this exist as a helper anywhere in the kernel? I don't really like >> XFS poking into the rw_semaphore innards, though I concede that this >> lock transferring dance is probably pretty rare. > I'll try to look for it. > Other code I see just calls rwsem_release directly - ocfs2, jbd2, kernfs etc. I think a clearer comment might suffice, not sure what Darrick thinks, maybe something like this: + /* + * Let lockdep know that we won't own i_lock when we hand off + * to the worker thread + */ + rwsem_release(&cur->bc_ino.ip->i_lock.dep_map, _THIS_IP_); queue_work(xfs_alloc_wq, &args.work); + wait_for_completion(&done); + /* We own the i_lock again */ + rwsem_acquire(&cur->bc_ino.ip->i_lock.dep_map, 0, 0, _RET_IP_); and similar comments in the worker: + /* Let lockdep know that we own the i_lock for now */ + rwsem_acquire(&args->cur->bc_ino.ip->i_lock.dep_map, 0, 0, _RET_IP_); ... etc -Eric
