aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/w1/w1.c
diff options
context:
space:
mode:
authorNeilBrown <neilb@suse.de>2012-05-18 01:59:52 -0400
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2012-06-13 19:38:40 -0400
commitb02f8bede217a4b145ecc16d3940c78d83941147 (patch)
treecbd02191cc2698c460b9abedea44de13855510c6 /drivers/w1/w1.c
parentb7e938d06d0de43bdbee8844a8736c81480c1031 (diff)
W1: split master mutex to avoid deadlocks.
The 'mutex' in struct w1_master is use for two very different purposes. Firstly it protects various data structures such as the list of all slaves. Secondly it protects the w1 buss against concurrent accesses. This can lead to deadlocks when the ->probe code called while adding a slave needs to talk on the bus, as is the case for power_supply devices. ds2780 and ds2781 drivers contain a work around to track which process hold the lock simply to avoid this deadlock. bq27000 doesn't have that work around and so deadlocks. There are other possible deadlocks involving sysfs. When removing a device the sysfs s_active lock is held, so the lock that protects the slave list must take precedence over s_active. However when access power_supply attributes via sysfs, the s_active lock must take precedence over the lock that protects accesses to the bus. So to avoid deadlocks between w1 slaves and sysfs, these must be two separate locks. Making them separate means that the work around in ds2780 and ds2781 can be removed. So this patch: - adds a new mutex: "bus_mutex" which serialises access to the bus. - takes in mutex in w1_search and ds1wm_search while they access the bus for searching. The mutex is dropped before calling the callback which adds the slave. - changes all slaves to use bus_mutex instead of mutex to protect access to the bus - removes w1_ds2790_io_nolock and w1_ds2781_io_nolock, and the related code from drivers/power/ds278[01]_battery.c which calls them. Signed-off-by: NeilBrown <neilb@suse.de> Acked-by: Evgeniy Polyakov <zbr@ioremap.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/w1/w1.c')
-rw-r--r--drivers/w1/w1.c9
1 files changed, 8 insertions, 1 deletions
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index bfb898641029..1a574370d2cd 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -885,16 +885,21 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
885 * 885 *
886 * Return 0 - device(s) present, 1 - no devices present. 886 * Return 0 - device(s) present, 1 - no devices present.
887 */ 887 */
888 mutex_lock(&dev->bus_mutex);
888 if (w1_reset_bus(dev)) { 889 if (w1_reset_bus(dev)) {
890 mutex_unlock(&dev->bus_mutex);
889 dev_dbg(&dev->dev, "No devices present on the wire.\n"); 891 dev_dbg(&dev->dev, "No devices present on the wire.\n");
890 break; 892 break;
891 } 893 }
892 894
893 /* Do fast search on single slave bus */ 895 /* Do fast search on single slave bus */
894 if (dev->max_slave_count == 1) { 896 if (dev->max_slave_count == 1) {
897 int rv;
895 w1_write_8(dev, W1_READ_ROM); 898 w1_write_8(dev, W1_READ_ROM);
899 rv = w1_read_block(dev, (u8 *)&rn, 8);
900 mutex_unlock(&dev->bus_mutex);
896 901
897 if (w1_read_block(dev, (u8 *)&rn, 8) == 8 && rn) 902 if (rv == 8 && rn)
898 cb(dev, rn); 903 cb(dev, rn);
899 904
900 break; 905 break;
@@ -927,10 +932,12 @@ void w1_search(struct w1_master *dev, u8 search_type, w1_slave_found_callback cb
927 rn |= (tmp64 << i); 932 rn |= (tmp64 << i);
928 933
929 if (kthread_should_stop()) { 934 if (kthread_should_stop()) {
935 mutex_unlock(&dev->bus_mutex);
930 dev_dbg(&dev->dev, "Abort w1_search\n"); 936 dev_dbg(&dev->dev, "Abort w1_search\n");
931 return; 937 return;
932 } 938 }
933 } 939 }
940 mutex_unlock(&dev->bus_mutex);
934 941
935 if ( (triplet_ret & 0x03) != 0x03 ) { 942 if ( (triplet_ret & 0x03) != 0x03 ) {
936 if ( (desc_bit == last_zero) || (last_zero < 0)) 943 if ( (desc_bit == last_zero) || (last_zero < 0))