| Message ID | 20220131165926.3230642-2-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix header test and log spam on !x86 | expand |
On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote: > Only x86 and in some cases PPC have support added in drm_cache.c for the > clflush class of functions. However warning once is sufficient to taint > the log instead of spamming it with "Architecture has no drm_cache.c > support" every few millisecond. Switch to WARN_ONCE() so we still get > the log message, but only once, together with the warning. E.g: > > ------------[ cut here ]------------ > Architecture has no drm_cache.c support > WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] > ... > > v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err() Reviewed-by: José Roberto de Souza <jose.souza@intel.com> But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? Anyways, it is better than before. > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > > v3: No changes from previous version, just submitting to the right > mailing list > > drivers/gpu/drm/drm_cache.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c > index f19d9acbe959..2c3fa5677f7e 100644 > --- a/drivers/gpu/drm/drm_cache.c > +++ b/drivers/gpu/drm/drm_cache.c > @@ -112,8 +112,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages) > kunmap_atomic(page_virtual); > } > #else > - pr_err("Architecture has no drm_cache.c support\n"); > - WARN_ON_ONCE(1); > + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif > } > EXPORT_SYMBOL(drm_clflush_pages); > @@ -143,8 +142,7 @@ drm_clflush_sg(struct sg_table *st) > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > #else > - pr_err("Architecture has no drm_cache.c support\n"); > - WARN_ON_ONCE(1); > + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif > } > EXPORT_SYMBOL(drm_clflush_sg); > @@ -177,8 +175,7 @@ drm_clflush_virt_range(void *addr, unsigned long length) > if (wbinvd_on_all_cpus()) > pr_err("Timed out waiting for cache flush\n"); > #else > - pr_err("Architecture has no drm_cache.c support\n"); > - WARN_ON_ONCE(1); > + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); > #endif > } > EXPORT_SYMBOL(drm_clflush_virt_range);
On Tue, Feb 01, 2022 at 09:12:05AM -0800, Jose Souza wrote: >On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote: >> Only x86 and in some cases PPC have support added in drm_cache.c for the >> clflush class of functions. However warning once is sufficient to taint >> the log instead of spamming it with "Architecture has no drm_cache.c >> support" every few millisecond. Switch to WARN_ONCE() so we still get >> the log message, but only once, together with the warning. E.g: >> >> ------------[ cut here ]------------ >> Architecture has no drm_cache.c support >> WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] >> ... >> >> v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err() > >Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > >But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? >Anyways, it is better than before. I thought about that, but it didn't seem justifiable because: 1) drm_WARN_ONCE will basically add dev_driver_string() to the log. However the warning message here is basically helping the bootstrap of additional archs. They shouldn't be seen on anything properly supported. 2) This seems all to be a layer below drm anyway and could even be used in places outside easy access to a drm pointer. So, it seems the benefit of using the subsystem-specific drm_WARN_ONCE doesn't justify the hassle of changing the callers, possibly adding additional back pointers to have access to the drm device pointer. thanks Lucas De Marchi
On Tue, Feb 01, 2022 at 09:41:33AM -0800, Lucas De Marchi wrote: > On Tue, Feb 01, 2022 at 09:12:05AM -0800, Jose Souza wrote: > > On Mon, 2022-01-31 at 08:59 -0800, Lucas De Marchi wrote: > > > Only x86 and in some cases PPC have support added in drm_cache.c for the > > > clflush class of functions. However warning once is sufficient to taint > > > the log instead of spamming it with "Architecture has no drm_cache.c > > > support" every few millisecond. Switch to WARN_ONCE() so we still get > > > the log message, but only once, together with the warning. E.g: > > > > > > ------------[ cut here ]------------ > > > Architecture has no drm_cache.c support > > > WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] > > > ... > > > > > > v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err() > > > > Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > > > But while at it, why not add a drm_device parameter to this function so we can use drm_WARN_ONCE()? > > Anyways, it is better than before. > > I thought about that, but it didn't seem justifiable because: > > 1) drm_WARN_ONCE will basically add dev_driver_string() to the log. > However the warning message here is basically helping the bootstrap of > additional archs. They shouldn't be seen on anything properly supported. > > 2) This seems all to be a layer below drm anyway and could even be used > in places outside easy access to a drm pointer. > > So, it seems the benefit of using the subsystem-specific drm_WARN_ONCE > doesn't justify the hassle of changing the callers, possibly adding > additional back pointers to have access to the drm device pointer. Initially I had same feeling as Jose, but good points raised here. Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > thanks > Lucas De Marchi
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c index f19d9acbe959..2c3fa5677f7e 100644 --- a/drivers/gpu/drm/drm_cache.c +++ b/drivers/gpu/drm/drm_cache.c @@ -112,8 +112,7 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages) kunmap_atomic(page_virtual); } #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_pages); @@ -143,8 +142,7 @@ drm_clflush_sg(struct sg_table *st) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_sg); @@ -177,8 +175,7 @@ drm_clflush_virt_range(void *addr, unsigned long length) if (wbinvd_on_all_cpus()) pr_err("Timed out waiting for cache flush\n"); #else - pr_err("Architecture has no drm_cache.c support\n"); - WARN_ON_ONCE(1); + WARN_ONCE(1, "Architecture has no drm_cache.c support\n"); #endif } EXPORT_SYMBOL(drm_clflush_virt_range);
Only x86 and in some cases PPC have support added in drm_cache.c for the clflush class of functions. However warning once is sufficient to taint the log instead of spamming it with "Architecture has no drm_cache.c support" every few millisecond. Switch to WARN_ONCE() so we still get the log message, but only once, together with the warning. E.g: ------------[ cut here ]------------ Architecture has no drm_cache.c support WARNING: CPU: 80 PID: 888 at drivers/gpu/drm/drm_cache.c:139 drm_clflush_sg+0x40/0x50 [drm] ... v2 (Jani): use WARN_ONCE() and keep the message previously on pr_err() Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> --- v3: No changes from previous version, just submitting to the right mailing list drivers/gpu/drm/drm_cache.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)