Re: [PATCH] nbd: do not reinitialize a live socket lock when reclassifying

[Aibing Chen <chenaibing@cmss.chinamobile.com>]

Hi Bart,

Thanks for the detailed review - all three points are valid, and I
agree the reclassification approach cannot be made safe for a live
socket:

1. Right - sock_allow_reclassification() is an unsynchronized check
   and lockdep_init_map_type() clears lock->class_cache[] without
   synchronization.  I also missed that lock_sock_nested() keeps
   sk->sk_lock.dep_map acquired in lockdep while sleeping in
   __lock_sock(), so reinitializing the dep_map while another task
   holds it breaks match_held_lock().

2. Right - sk_owner_set() calls __module_get() unconditionally, so
   repeated NBD_SET_SOCK on the same fd leaks module references.

3. Understood: as long as userspace can still issue sendmsg() on the
   socket, the original sk_lock -> mmap_lock -> fs_reclaim cycle is
   recreated regardless of the lock class, so reclassification does
   not fix what d532cddb6c60 aimed at.

I will drop this patch.  Are you (or Eric) planning to prepare the
revert of d532cddb6c60 and 9280e6edf656 together with the TX/shutdown
deferral to a workqueue, or would you like me to prepare it?  If you
would like me to prepare it, I would appreciate your guidance on the
workqueue design (nbd_queue_rq() currently transmits inline via
nbd_handle_cmd()).

Thanks again,

Aibing Chen