On Wed, Oct 13, 2021 at 09:07:03AM +0800, Ming Lei wrote: > On Tue, Oct 12, 2021 at 02:18:28PM -0700, Luis Chamberlain wrote: > > > Looks test_sysfs isn't in linus tree, where can I find it? > > > > https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20210927-sysfs-generic-deadlock-fix > > > > To reproduce the deadlock revert the patch in this thread and then run > > either of these two tests as root: > > > > ./tools/testing/selftests/sysfs/sysfs.sh -w 0027 > > ./tools/testing/selftests/sysfs/sysfs.sh -w 0028 > > > > You will need to enable the test_sysfs driver. > > > Can you share the code which waits for the sysfs / kernfs files to be > > > stop being used? > > > > How about a call trace of the two tasks which deadlock, here is one of > > running test 0027: > > > > kdevops login: [ 363.875459] INFO: task sysfs.sh:1271 blocked for more > > than 120 seconds. <-- snip --> > That doesn't show the deadlock is related with module_exit(). Not directly no. > It is clearly one AA deadlock, what I meant was that it isn't related with > module exit cause lock & device_del() isn't always done in module exit, so > I doubt your fix with grabbing module refcnt is good or generic enough. A device_del() *can* happen in other areas other than module exit sure, but the issue is if a shared lock is used *before* device_del() and also used on a sysfs op. Typically this can happen on module exit, and the other common use case in my experience is on sysfs ops, such is the case with the zram driver. Both cases are covered then by this fix. If there are other areas, that is still driver specific, but of the things we *can* generalize, definitely module exit is a common path. > Except for your cooked test_sys module, how many real drivers do suffer the > problem? What are they? I only really seriously considered trying to generalize this after it was hinted to me live patching was also affected, and so clearly something generic was desirable. There may be other drivers for sure, but a hunt for that with semantics would require a bit complex coccinelle patch with iteration support. > Why can't we fix the exact driver? You can try, the way the lock is used in zram is correct, specially after my other fix in this series which addresses another unrelated bug with cpu hotplug multistate support. So we then can proceed to either take the position to say: "Thou shalt not use a shared lock on module exit and a sysfs op" and try to fix all places, or we generalize a fix for this. A generic fix seems more desirable. Luis
