| Message ID | 20190118180717.163547-2-hoegsberg@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm: Add DRM_DEV_INFO_RATELIMITED | expand |
On Fri, 18 Jan 2019, "Kristian H. Kristensen" <hoegsberg@gmail.com> wrote: > Otherwise we get hard to track down "Purging: 123123 bytes" messages in > the log. > > Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org> > --- > drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c > index b72d8e6cd51d7..8161923892f55 100644 > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > mutex_unlock(&dev->struct_mutex); > > if (freed > 0) > - pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT); > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", freed << PAGE_SHIFT); I'm not opposed to the patches per se, but it does seem a bit odd to be printing info level messages in a way that might need ratelimiting. Is this a hint you should perhaps make it debug? BR, Jani. > > return freed; > } > @@ -134,7 +134,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) > *(unsigned long *)ptr += unmapped; > > if (unmapped > 0) > - pr_info_ratelimited("Purging %u vmaps\n", unmapped); > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %u vmaps\n", unmapped); > > return NOTIFY_DONE; > }
On Mon, Jan 21, 2019 at 1:36 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 18 Jan 2019, "Kristian H. Kristensen" <hoegsberg@gmail.com> wrote: > > Otherwise we get hard to track down "Purging: 123123 bytes" messages in > > the log. > > > > Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org> > > --- > > drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > index b72d8e6cd51d7..8161923892f55 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > > mutex_unlock(&dev->struct_mutex); > > > > if (freed > 0) > > - pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT); > > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", freed << PAGE_SHIFT); > > I'm not opposed to the patches per se, but it does seem a bit odd to be > printing info level messages in a way that might need ratelimiting. Is > this a hint you should perhaps make it debug? Yeah, that's a good point - maybe this just needs to be debug... Kristian > BR, > Jani. > > > > > > return freed; > > } > > @@ -134,7 +134,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) > > *(unsigned long *)ptr += unmapped; > > > > if (unmapped > 0) > > - pr_info_ratelimited("Purging %u vmaps\n", unmapped); > > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %u vmaps\n", unmapped); > > > > return NOTIFY_DONE; > > } > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, Jan 21, 2019 at 4:36 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Fri, 18 Jan 2019, "Kristian H. Kristensen" <hoegsberg@gmail.com> wrote: > > Otherwise we get hard to track down "Purging: 123123 bytes" messages in > > the log. > > > > Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org> > > --- > > drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > index b72d8e6cd51d7..8161923892f55 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c > > +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c > > @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) > > mutex_unlock(&dev->struct_mutex); > > > > if (freed > 0) > > - pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT); > > + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", freed << PAGE_SHIFT); > > I'm not opposed to the patches per se, but it does seem a bit odd to be > printing info level messages in a way that might need ratelimiting. Is > this a hint you should perhaps make it debug? > I'm probably to blame for it being info.. at least for "traditional" linux userspace, hitting the srinker hard on a device with a moderate amount of memory was kinda an abnormal situation and something I wanted to at least be aware of in potential bug reports.. I guess since it seems to be more a "business as usual" situation on android other such userspaces, maybe we should demote this to debug (but ime it should still be ratelimited) BR, -R
diff --git a/drivers/gpu/drm/msm/msm_gem_shrinker.c b/drivers/gpu/drm/msm/msm_gem_shrinker.c index b72d8e6cd51d7..8161923892f55 100644 --- a/drivers/gpu/drm/msm/msm_gem_shrinker.c +++ b/drivers/gpu/drm/msm/msm_gem_shrinker.c @@ -98,7 +98,7 @@ msm_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc) mutex_unlock(&dev->struct_mutex); if (freed > 0) - pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT); + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %lu bytes\n", freed << PAGE_SHIFT); return freed; } @@ -134,7 +134,7 @@ msm_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr) *(unsigned long *)ptr += unmapped; if (unmapped > 0) - pr_info_ratelimited("Purging %u vmaps\n", unmapped); + DRM_DEV_INFO_RATELIMITED(dev->dev, "Purging %u vmaps\n", unmapped); return NOTIFY_DONE; }
Otherwise we get hard to track down "Purging: 123123 bytes" messages in the log. Signed-off-by: Kristian H. Kristensen <hoegsberg@chromium.org> --- drivers/gpu/drm/msm/msm_gem_shrinker.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)