Commit 789dd8cb authored by Gerald Schaefer's avatar Gerald Schaefer Committed by Heiko Carstens

s390/dcssblk: fix lockdep warning

dcssblk_remove_store() holds the dcssblk_devices_sem semaphore while
calling del_gendisk(dev_info->gd), which in turn tries to acquire
disk->open_mutex. Then there is dcssblk_release(), which is called
with disk->open_mutex held, and tries to acquire dcssblk_devices_sem.

Lockdep reports this as possible circular locking dependency (CPU0 =
dcssblk_remove_store, CPU1 = dcssblk_release):

[   44.948865]  Possible unsafe locking scenario:

[   44.948866]        CPU0                    CPU1
[   44.948867]        ----                    ----
[   44.948868]   lock(&dcssblk_devices_sem);
[   44.948870]                                lock(&disk->open_mutex);
[   44.948872]                                lock(&dcssblk_devices_sem);
[   44.948874]   lock(&disk->open_mutex);
[   44.948876]
                *** DEADLOCK ***

In practice, this deadlock should not happen, since dcssblk_remove_store()
checks for dev_info->use_count != 0 after acquiring dcssblk_devices_sem,
and breaks out before calling del_gendisk(). dev_info->use_count will be
decremented in dcssblk_release(), protected by dcssblk_devices_sem.

Still there is no need for dcssblk_remove_store() to hold the
dcssblk_devices_sem until after calling del_gendisk(), as this only
protects dcssblk internal data. So fix the lockdep warning by releasing
dcssblk_devices_sem earlier. Also move the segment_unload() loop up,
similar to dcssblk_shared_store() error path, no need to do that after
calling del_gendisk().

Also change dcssblk_shared_store() error path, where dcssblk_devices_sem
was also released only after calling del_gendisk(), and a similar lockdep
warning could be triggered (but also deadlock prevented by check for
dev_info->use_count).
Acked-by: default avatarHeiko Carstens <hca@linux.ibm.com>
Signed-off-by: default avatarGerald Schaefer <gerald.schaefer@linux.ibm.com>
Signed-off-by: default avatarHeiko Carstens <hca@linux.ibm.com>
parent 67ce50ce
...@@ -411,13 +411,13 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch ...@@ -411,13 +411,13 @@ dcssblk_shared_store(struct device *dev, struct device_attribute *attr, const ch
segment_unload(entry->segment_name); segment_unload(entry->segment_name);
} }
list_del(&dev_info->lh); list_del(&dev_info->lh);
up_write(&dcssblk_devices_sem);
dax_remove_host(dev_info->gd); dax_remove_host(dev_info->gd);
kill_dax(dev_info->dax_dev); kill_dax(dev_info->dax_dev);
put_dax(dev_info->dax_dev); put_dax(dev_info->dax_dev);
del_gendisk(dev_info->gd); del_gendisk(dev_info->gd);
put_disk(dev_info->gd); put_disk(dev_info->gd);
up_write(&dcssblk_devices_sem);
if (device_remove_file_self(dev, attr)) { if (device_remove_file_self(dev, attr)) {
device_unregister(dev); device_unregister(dev);
...@@ -790,18 +790,17 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch ...@@ -790,18 +790,17 @@ dcssblk_remove_store(struct device *dev, struct device_attribute *attr, const ch
} }
list_del(&dev_info->lh); list_del(&dev_info->lh);
/* unload all related segments */
list_for_each_entry(entry, &dev_info->seg_list, lh)
segment_unload(entry->segment_name);
up_write(&dcssblk_devices_sem);
dax_remove_host(dev_info->gd); dax_remove_host(dev_info->gd);
kill_dax(dev_info->dax_dev); kill_dax(dev_info->dax_dev);
put_dax(dev_info->dax_dev); put_dax(dev_info->dax_dev);
del_gendisk(dev_info->gd); del_gendisk(dev_info->gd);
put_disk(dev_info->gd); put_disk(dev_info->gd);
/* unload all related segments */
list_for_each_entry(entry, &dev_info->seg_list, lh)
segment_unload(entry->segment_name);
up_write(&dcssblk_devices_sem);
device_unregister(&dev_info->dev); device_unregister(&dev_info->dev);
put_device(&dev_info->dev); put_device(&dev_info->dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment