diff mbox series

[5/5] drm/i915: switch intel_wakeref_t underlying type to struct ref_tracker *

Message ID 067332b1f8679f940f373618fd24b1d03370ba45.1726658138.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: wakeref fixes and improvements | expand

Commit Message

Jani Nikula Sept. 18, 2024, 11:17 a.m. UTC 
For intel_wakeref_t, opaque is reasonable, but disguising the underlying
struct ref_tracker * as an unsigned long is not so great. Update the
typedef to remove one level of disguise.

Although the kernel coding style strongly discourages pointer typedefs,
it's a better alternative, and an incremental improvement on the status
quo. It provides much better type safety than an unsigned long could,
and prevents passing magic -1 instead of INTEL_WAKEREF_DEF. Moreover, it
provides a gradual path for replacing intel_wakeref_t with struct
ref_tracker * if desired.

As an extra safety measure, check for error pointers in
intel_ref_tracker_free() before passing them on to ref_tracker_free(),
to catch any mistakes with mock gt special wakeref value.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.h            |  2 +-
 drivers/gpu/drm/i915/intel_wakeref.h             | 16 +++++++++-------
 .../drm/xe/compat-i915-headers/intel_wakeref.h   |  4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)

Comments

kernel test robot Sept. 18, 2024, 4:23 p.m. UTC | #1 
Hi Jani,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-xe/drm-xe-next]
[also build test WARNING on next-20240918]
[cannot apply to drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.11]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-i915-gem-fix-bitwise-and-logical-AND-mixup/20240918-191938
base:   https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next
patch link:    https://lore.kernel.org/r/067332b1f8679f940f373618fd24b1d03370ba45.1726658138.git.jani.nikula%40intel.com
patch subject: [PATCH 5/5] drm/i915: switch intel_wakeref_t underlying type to struct ref_tracker *
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240919/202409190032.ZCHBxK9e-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240919/202409190032.ZCHBxK9e-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409190032.ZCHBxK9e-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_display_power.c:548:10: warning: expression which evaluates to zero treated as a null pointer constant of type 'intel_wakeref_t' (aka 'struct ref_tracker *') [-Wnon-literal-null-conversion]
     548 |                 return false;
         |                        ^~~~~
   1 warning generated.


vim +548 drivers/gpu/drm/i915/display/intel_display_power.c

7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  525  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  526  /**
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  527   * intel_display_power_get_if_enabled - grab a reference for an enabled display power domain
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  528   * @dev_priv: i915 device instance
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  529   * @domain: power domain to reference
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  530   *
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  531   * This function grabs a power domain reference for @domain and ensures that the
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  532   * power domain and all its parents are powered up. Therefore users should only
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  533   * grab a reference to the innermost power domain they need.
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  534   *
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  535   * Any power domain reference obtained by this function must have a symmetric
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  536   * call to intel_display_power_put() to release the reference again.
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  537   */
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  538  intel_wakeref_t
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  539  intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  540  				   enum intel_display_power_domain domain)
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  541  {
e3e8148f43fb6d drivers/gpu/drm/i915/display/intel_display_power.c Jani Nikula            2022-08-29  542  	struct i915_power_domains *power_domains = &dev_priv->display.power.domains;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  543  	intel_wakeref_t wakeref;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  544  	bool is_enabled;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  545  
d858d5695f3897 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-06-13  546  	wakeref = intel_runtime_pm_get_if_in_use(&dev_priv->runtime_pm);
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  547  	if (!wakeref)
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31 @548  		return false;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  549  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  550  	mutex_lock(&power_domains->lock);
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  551  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  552  	if (__intel_display_power_is_enabled(dev_priv, domain)) {
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  553  		__intel_display_power_get_domain(dev_priv, domain);
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  554  		is_enabled = true;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  555  	} else {
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  556  		is_enabled = false;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  557  	}
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  558  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  559  	mutex_unlock(&power_domains->lock);
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  560  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  561  	if (!is_enabled) {
d858d5695f3897 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-06-13  562  		intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  563  		wakeref = 0;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  564  	}
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  565  
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  566  	return wakeref;
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  567  }
7645b19d9bdd13 drivers/gpu/drm/i915/intel_display_power.c         Daniele Ceraolo Spurio 2019-05-31  568
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
index fef8d5d288f8..dcbfc09194b7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
@@ -105,7 +105,7 @@  int intel_gt_runtime_resume(struct intel_gt *gt);
 
 ktime_t intel_gt_get_awake_time(const struct intel_gt *gt);
 
-#define INTEL_WAKEREF_MOCK_GT ((intel_wakeref_t)-ENODEV)
+#define INTEL_WAKEREF_MOCK_GT ERR_PTR(-ENODEV)
 
 static inline bool is_mock_gt(const struct intel_gt *gt)
 {
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h
index 3944587a5e78..48836ef52d40 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -21,7 +21,7 @@ 
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 
-typedef unsigned long intel_wakeref_t;
+typedef struct ref_tracker *intel_wakeref_t;
 
 #define INTEL_REFTRACK_DEAD_COUNT 16
 #define INTEL_REFTRACK_PRINT_LIMIT 16
@@ -273,7 +273,7 @@  __intel_wakeref_defer_park(struct intel_wakeref *wf)
  */
 int intel_wakeref_wait_for_idle(struct intel_wakeref *wf);
 
-#define INTEL_WAKEREF_DEF ((intel_wakeref_t)(-1))
+#define INTEL_WAKEREF_DEF ERR_PTR(-ENOENT)
 
 static inline intel_wakeref_t intel_ref_tracker_alloc(struct ref_tracker_dir *dir)
 {
@@ -281,17 +281,19 @@  static inline intel_wakeref_t intel_ref_tracker_alloc(struct ref_tracker_dir *di
 
 	ref_tracker_alloc(dir, &user, GFP_NOWAIT);
 
-	return (intel_wakeref_t)user ?: INTEL_WAKEREF_DEF;
+	return user ?: INTEL_WAKEREF_DEF;
 }
 
 static inline void intel_ref_tracker_free(struct ref_tracker_dir *dir,
-					  intel_wakeref_t handle)
+					  intel_wakeref_t wakeref)
 {
-	struct ref_tracker *user;
+	if (wakeref == INTEL_WAKEREF_DEF)
+		wakeref = NULL;
 
-	user = (handle == INTEL_WAKEREF_DEF) ? NULL : (void *)handle;
+	if (WARN_ON(IS_ERR(wakeref)))
+		return;
 
-	ref_tracker_free(dir, &user);
+	ref_tracker_free(dir, &wakeref);
 }
 
 void intel_ref_tracker_show(struct ref_tracker_dir *dir,
diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h b/drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h
index 5c139ba144a6..2a32faea9db5 100644
--- a/drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h
+++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_wakeref.h
@@ -5,6 +5,6 @@ 
 
 #include <linux/types.h>
 
-typedef unsigned long intel_wakeref_t;
+typedef struct ref_tracker *intel_wakeref_t;
 
-#define INTEL_WAKEREF_DEF ((intel_wakeref_t)(-1))
+#define INTEL_WAKEREF_DEF ERR_PTR(-ENOENT)