On Tue, Oct 19, 2021 at 08:23:51AM +0200, Miroslav Benes wrote:
By you only addressing the deadlock as a requirement on approach a) you are forgetting that there *may* already be present drivers which *do* implement such patterns in the kernel. I worked on addressing the deadlock because I was informed livepatching *did* have that issue as well and so very likely a generic solution to the deadlock could be beneficial to other random drivers.
In-tree zram doesn't have such deadlock, if livepatching has such AA deadlock, just fixed it, and seems it has been fixed by 3ec24776bfd0.
I would not call it a fix. It is a kind of ugly workaround because the generic infrastructure lacked (lacks) the proper support in my opinion. Luis is trying to fix that.
What is the proper support of the generic infrastructure? I am not familiar with livepatching's model(especially with module unload), you mean livepatching have to do the following way from sysfs:
1) during module exit: mutex_lock(lp_lock); kobject_put(lp_kobj); mutex_unlock(lp_lock); 2) show()/store() method of attributes of lp_kobj mutex_lock(lp_lock) ... mutex_unlock(lp_lock)
IMO, the above usage simply caused AA deadlock. Even in Luis's patch 'zram: fix crashes with cpu hotplug multistate', new/same AA deadlock (hot_remove_store() vs. disksize_store() or reset_store()) is added because hot_remove_store() isn't called from module_exit().
Luis tries to delay unloading module until all show()/store() are done. But that can be obtained by the following way simply during module_exit():
kobject_del(lp_kobj); //all pending store()/show() from lp_kobj are done, //no new store()/show() can come after //kobject_del() returns mutex_lock(lp_lock); kobject_put(lp_kobj); mutex_unlock(lp_lock);
Or can you explain your requirement on kobject/module unload in a bit details?
Thanks, Ming