On Tue, Feb 28, 2017 at 1:33 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Tue, Feb 28, 2017 at 01:28:13PM +0000, Matthew Auld wrote: >> On 22 February 2017 at 15:25, Robert Bragg <robert@xxxxxxxxxxxxx> wrote: >> > This change is pre-emptively aiming to avoid a potential cause of kernel >> > logging noise in case some condition were to result in us seeing invalid >> > OA reports. >> > >> > The workaround for the OA unit's tail pointer race condition is what >> > avoids the primary know cause of invalid reports being seen and with >> > that in place we aren't expecting to see this notice but it can't be >> > entirely ruled out. >> > >> > Just in case some condition does lead to the notice then it's likely >> > that it will be triggered repeatedly while attempting to append a >> > sequence of reports and depending on the configured OA sampling >> > frequency that might be a large number of repeat notices. >> > >> > Signed-off-by: Robert Bragg <robert@xxxxxxxxxxxxx> >> Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx> > > printk_ratelimits emits a WARN when triggered, defeating the purpose of > using NOTE. Hmm, that's a slightly awkward problem. The warning comes from the common __ratelimit utility api that's used for numerous things but by default will warn if the rate limiting kicked in (once printing resumes again). There's a RATELIMIT_MSG_ON_RELEASE flag that can be set to def the warning until ratelimit_state_exit() is called (which looks optional or at least the flag could also be cleared before calling it to avoid any warnings). In general printk_ratelimit() doesn't seem like an ideal mechanism for throttling messages, even if they are warnings, since the rate limit state is shared across orthogonal components, so maybe it's best anyway to use a custom rate limit state. Considering the _MSG_ON_RELEASE flag, I think I'll init some ratelimit state in dev_priv->perf.oa just for this message, use ratelimit_set_flags() to set _MSG_ON_RELEASE and have a corresponding debug/note in i915_perf_fini after checking the rs->missed counter. Actually at this point I'm slightly doubting whether having a warning for throttling is that terrible in this case. Although I tend to think it could be good to have dedicated ratelimit state here, there conceptually is a point a which a visible warning could be appropriate if we're really seeing a lot of these spurious reports. It's a grey area since it's ok as note if it's rare/intermittent, but somthing's maybe gone wrong if we see *lots* of these. hmm. Incidentally, I suppose this same observation is relevant to many of the DRM_*_RATELIMITED macros too - there will be an inconsistent level used to notify about any throttling? Br, - Robert > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
