Re: possible circular locking dependency detected: compound_send_recv+0x189/0x910 [cifs] vm_mmap_pgoff+0x85/0x160

[Vincent Whitchurch]

On Sat, Aug 28, 2021 at 04:02:36AM +0200, Hillf Danton wrote:
> On Fri, 27 Aug 2021 15:27:24 +0200 Vincent Whitchurch wrote:
> > On Fri, Aug 27, 2021 at 10:27:46AM +0200, Hillf Danton wrote:
> > > 
> > > Only if it is too difficult to fix 05946d4b7a73 ("cifs: Fix preauth hash
> > > corruption") within cifs then fix the deadlock by replacing kthread_run()
> > > with queue_work().
> > 
> > Perhaps I'm missing something, but would the lockdep complaint really go
> > away without 05946d4b7a73?  cifs_alloc_hash() is called under the
> > srv_mutex in other places (for example setup_ntlmv2_rsp()), so the
> > 
> > 	&tcp_ses->srv_mutex --> &cpuset_rwsem --> &mm->mmap_lock
> > 
> > chain would still exist, and compound_send_recv() takes srv_mutex before
> > 05946d4b7a73 too, so &mm->mmap_lock -> srv_mutex would exist too.
> 
> Yes you are right. The key is mmap_lock here.
> > 
> > For cifs_alloc_hash() to be able to be called without the srv_mutex I
> > guess it would have to be done when the tcp_ses is allocated.  That
> > however would essentially be a revert of commit 95dc8dd14e2e84cc3ada
> > ("Limit allocation of crypto mechanisms to dialect which requires").
> 
> It is more appreciated to have a fix within cifs.

Yes.  I'm hoping someone else with more insight into the cifs code can
see if there's another way to fix this in cifs or if it's safe to try
and revert 95dc8dd14e2e84cc3ada.  Steve?