> > > > - atomic_inc(&lo->lo_refcnt); > > > > -out: > > > > + err = mutex_lock_killable(&lo->lo_mutex); > > > > mutex_unlock(&loop_ctl_mutex); > > > > > > I don't see a possibility for deadlock but it bothers me a little that > > > we're not unlocking in the reverse locking order here, as we do in > > > loop_control_ioctl(). There should be no perf impact if we move the > > > mutex_unlock(&loop_ctl_mutex) after mutex_unlock(&lo->lo_mutex). > > > > The lo_open() was one of the top functions that showed up in > > contention profiling, and the only shared data that it updates is > > lo_recnt which can be protected by lo_mutex. We must have > > loop_ctl_mutex in order to get a valid lo pointer, otherwise we could > > race with loop_control_ioctl(LOOP_CTL_REMOVE). Unlocking in a > > different order is not an issue, as long as we always preserve the > > locking order. > > It is probably a good idea to leave a comment about this in the > lo_open() so that nobody comes along and tries to "correct" the > unlocking order in the future and, as a result, introduces a perf > regression. > Makes sense, I will add a comment about it. Thank you, Pasha
