| Message ID | 20200402055241.24789-1-pankaj.laxminarayan.bharadiya@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove WARN_ON and WARN_ON_ONCE overrides. | expand |
On Thu, 02 Apr 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote: > Now we have new struct drm_device based drm_WARN* macros. These are > preferred over the regular WARN* macros. > > Remove WARN_ON and WARN_ON_ONCE overriedes to avoid any temptations to > use them in the future. Well, since they are overrides of macros in bug.h, the users are still there. There are still 100+ users in i915. You just don't get as much information with them after this patch. I'm not opposed to this patch, but at the same time I'd like to see more converted to the drm_WARN* alternatives. Reviewed-by: Jani Nikula <jani.nikula@intel.com> BR, Jani. > > Suggested-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> > --- > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > index 03a73d2bd50d..c666a64375d9 100644 > --- a/drivers/gpu/drm/i915/i915_utils.h > +++ b/drivers/gpu/drm/i915/i915_utils.h > @@ -36,21 +36,6 @@ struct timer_list; > > #define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs" > > -#undef WARN_ON > -/* Many gcc seem to no see through this and fall over :( */ > -#if 0 > -#define WARN_ON(x) ({ \ > - bool __i915_warn_cond = (x); \ > - if (__builtin_constant_p(__i915_warn_cond)) \ > - BUILD_BUG_ON(__i915_warn_cond); \ > - WARN(__i915_warn_cond, "WARN_ON(" #x ")"); }) > -#else > -#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")") > -#endif > - > -#undef WARN_ON_ONCE > -#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")") > - > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > __stringify(x), (long)(x))
On Thu, Apr 2, 2020 at 8:23 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Thu, 02 Apr 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote: > > Now we have new struct drm_device based drm_WARN* macros. These are > > preferred over the regular WARN* macros. > > > > Remove WARN_ON and WARN_ON_ONCE overriedes to avoid any temptations to > > use them in the future. > > Well, since they are overrides of macros in bug.h, the users are still > there. There are still 100+ users in i915. You just don't get as much > information with them after this patch. > > I'm not opposed to this patch, but at the same time I'd like to see more > converted to the drm_WARN* alternatives. Yeah our overrides are super useful, I think better to try to move them to the core version. So many times when I didn't know which warning was hit in a bug report (warnings in drm core already have this problem), and the additional output here would have helped. I'd at least keep this for now. -Daniel > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > > BR, > Jani. > > > > > Suggested-by: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> > > --- > > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- > > 1 file changed, 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > > index 03a73d2bd50d..c666a64375d9 100644 > > --- a/drivers/gpu/drm/i915/i915_utils.h > > +++ b/drivers/gpu/drm/i915/i915_utils.h > > @@ -36,21 +36,6 @@ struct timer_list; > > > > #define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs" > > > > -#undef WARN_ON > > -/* Many gcc seem to no see through this and fall over :( */ > > -#if 0 > > -#define WARN_ON(x) ({ \ > > - bool __i915_warn_cond = (x); \ > > - if (__builtin_constant_p(__i915_warn_cond)) \ > > - BUILD_BUG_ON(__i915_warn_cond); \ > > - WARN(__i915_warn_cond, "WARN_ON(" #x ")"); }) > > -#else > > -#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")") > > -#endif > > - > > -#undef WARN_ON_ONCE > > -#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")") > > - > > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ > > __stringify(x), (long)(x)) > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 02 Apr 2020, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Apr 2, 2020 at 8:23 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: >> >> On Thu, 02 Apr 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote: >> > Now we have new struct drm_device based drm_WARN* macros. These are >> > preferred over the regular WARN* macros. >> > >> > Remove WARN_ON and WARN_ON_ONCE overriedes to avoid any temptations to >> > use them in the future. >> >> Well, since they are overrides of macros in bug.h, the users are still >> there. There are still 100+ users in i915. You just don't get as much >> information with them after this patch. >> >> I'm not opposed to this patch, but at the same time I'd like to see more >> converted to the drm_WARN* alternatives. > > Yeah our overrides are super useful, I think better to try to move > them to the core version. So many times when I didn't know which > warning was hit in a bug report (warnings in drm core already have > this problem), and the additional output here would have helped. I'd > at least keep this for now. The drm_WARN_ON, drm_WARN_ON_ONCE macros in drm_print.h are comparable replacements, with the device info as well as the condition. BR, Jani. > -Daniel > >> Reviewed-by: Jani Nikula <jani.nikula@intel.com> >> >> >> BR, >> Jani. >> >> > >> > Suggested-by: Jani Nikula <jani.nikula@intel.com> >> > Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_utils.h | 15 --------------- >> > 1 file changed, 15 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h >> > index 03a73d2bd50d..c666a64375d9 100644 >> > --- a/drivers/gpu/drm/i915/i915_utils.h >> > +++ b/drivers/gpu/drm/i915/i915_utils.h >> > @@ -36,21 +36,6 @@ struct timer_list; >> > >> > #define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs" >> > >> > -#undef WARN_ON >> > -/* Many gcc seem to no see through this and fall over :( */ >> > -#if 0 >> > -#define WARN_ON(x) ({ \ >> > - bool __i915_warn_cond = (x); \ >> > - if (__builtin_constant_p(__i915_warn_cond)) \ >> > - BUILD_BUG_ON(__i915_warn_cond); \ >> > - WARN(__i915_warn_cond, "WARN_ON(" #x ")"); }) >> > -#else >> > -#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")") >> > -#endif >> > - >> > -#undef WARN_ON_ONCE >> > -#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")") >> > - >> > #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ >> > __stringify(x), (long)(x)) >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 03a73d2bd50d..c666a64375d9 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -36,21 +36,6 @@ struct timer_list; #define FDO_BUG_URL "https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs" -#undef WARN_ON -/* Many gcc seem to no see through this and fall over :( */ -#if 0 -#define WARN_ON(x) ({ \ - bool __i915_warn_cond = (x); \ - if (__builtin_constant_p(__i915_warn_cond)) \ - BUILD_BUG_ON(__i915_warn_cond); \ - WARN(__i915_warn_cond, "WARN_ON(" #x ")"); }) -#else -#define WARN_ON(x) WARN((x), "%s", "WARN_ON(" __stringify(x) ")") -#endif - -#undef WARN_ON_ONCE -#define WARN_ON_ONCE(x) WARN_ONCE((x), "%s", "WARN_ON_ONCE(" __stringify(x) ")") - #define MISSING_CASE(x) WARN(1, "Missing case (%s == %ld)\n", \ __stringify(x), (long)(x))
Now we have new struct drm_device based drm_WARN* macros. These are preferred over the regular WARN* macros. Remove WARN_ON and WARN_ON_ONCE overriedes to avoid any temptations to use them in the future. Suggested-by: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> --- drivers/gpu/drm/i915/i915_utils.h | 15 --------------- 1 file changed, 15 deletions(-)