[PATCH 2/2] platform/x86/intel-uncore-freq: Fix deadlock when removing attributes

[Srinivas Pandruvada]

uncore_freq_remove_die_entry() holds uncore_lock while removing the
attribute group of an entry. Removing attributes waits for active sysfs
callbacks of these attributes to finish. But the show and store callbacks
take uncore_lock, which results in an ABBA deadlock:

  CPU0                              CPU1
  ----                              ----
  kernfs_fop_read_iter()
    show_attr()                     uncore_freq_remove_die_entry()
                                      mutex_lock(&uncore_lock)
      mutex_lock(&uncore_lock)        delete_attr_group()
                                        sysfs_remove_group()
                                          waits for CPU0

Mark the entry as not valid with an invalid control CPU under
uncore_lock, and remove the attribute group after releasing it. Free
the seqnum_id after the attribute group is removed, so that the name
can't be used by a new entry while the old attributes still exist.

Adding and removing the same entry is serialized by the callers: CPU
hotplug callbacks in the MSR based driver, and device probe and remove
in the TPMI driver. Callbacks running before the attributes are removed
are safe: the MSR based driver returns -ENXIO for an invalid control CPU,
and the TPMI driver MMIO is unmapped only after the device is removed.

Fixes: ae7b2ce57851 ("platform/x86/intel/uncore-freq: Use sysfs API to create attributes")
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
---
 .../uncore-frequency-common.c                 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
index 6051aa040abf..22019099a857 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
@@ -313,17 +313,24 @@ void uncore_freq_remove_die_entry(struct uncore_data *data)
 {
 	mutex_lock(&uncore_lock);
 	/* Nothing to remove, if the entry was not added or already removed */
-	if (!data->valid)
-		goto uncore_unlock;
+	if (!data->valid) {
+		mutex_unlock(&uncore_lock);
+		return;
+	}
 
-	delete_attr_group(data, data->name);
 	data->control_cpu = -1;
 	data->valid = false;
+	mutex_unlock(&uncore_lock);
+
+	/*
+	 * Removing the attributes waits for active sysfs callbacks, which take
+	 * uncore_lock. So this can't be done while holding uncore_lock.
+	 */
+	delete_attr_group(data, data->name);
+
+	/* Free the ID after the attributes with the name using it are removed */
 	if (data->domain_id != UNCORE_DOMAIN_ID_INVALID)
 		ida_free(&intel_uncore_ida, data->seqnum_id);
-
-uncore_unlock:
-	mutex_unlock(&uncore_lock);
 }
 EXPORT_SYMBOL_NS_GPL(uncore_freq_remove_die_entry, "INTEL_UNCORE_FREQUENCY");
 
-- 
2.55.0