On Sat, Jun 29, 2013 at 11:26:50PM +0100, Chris Wilson wrote: > So it appears that I have encountered some bogosity when trying to call > i915_error_printf() with many arguments from print_error_buffers(). The > symptom is that the vsnprintf parser tries to interpret an integer arg > as a character string, the resulting OOPS indicating stack corruption. > Replacing the single call with its 13 format specifiers and arguments > with multiple calls to i915_error_printf() worked fine. This patch goes > one step further and introduced i915_error_puts() to pass the strings > simply. > > It may not fix the root cause, but it does prevent my box from dying and > I think helps make print_error_buffers() more friendly. > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66077 > Cc: Mika Kuoppala <mika.kuoppala at intel.com> Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_debugfs.c | 109 ++++++++++++++++++++++++++---------- > 1 file changed, 79 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 55c89af..7eae6b0 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -650,41 +650,44 @@ static const char *purgeable_flag(int purgeable) > return purgeable ? " purgeable" : ""; > } > > -static void i915_error_vprintf(struct drm_i915_error_state_buf *e, > - const char *f, va_list args) > +static bool __i915_error_ok(struct drm_i915_error_state_buf *e) > { > - unsigned len; > > if (!e->err && WARN(e->bytes > (e->size - 1), "overflow")) { > e->err = -ENOSPC; > - return; > + return false; > } > > if (e->bytes == e->size - 1 || e->err) > - return; > + return false; > > - /* Seek the first printf which is hits start position */ > - if (e->pos < e->start) { > - len = vsnprintf(NULL, 0, f, args); > - if (e->pos + len <= e->start) { > - e->pos += len; > - return; > - } > + return true; > +} > > - /* First vsnprintf needs to fit in full for memmove*/ > - if (len >= e->size) { > - e->err = -EIO; > - return; > - } > +static bool __i915_error_seek(struct drm_i915_error_state_buf *e, > + unsigned len) > +{ > + if (e->pos + len <= e->start) { > + e->pos += len; > + return false; > } > > - len = vsnprintf(e->buf + e->bytes, e->size - e->bytes, f, args); > - if (len >= e->size - e->bytes) > - len = e->size - e->bytes - 1; > + /* First vsnprintf needs to fit in its entirety for memmove */ > + if (len >= e->size) { > + e->err = -EIO; > + return false; > + } > > + return true; > +} > + > +static void __i915_error_advance(struct drm_i915_error_state_buf *e, > + unsigned len) > +{ > /* If this is first printf in this window, adjust it so that > * start position matches start of the buffer > */ > + > if (e->pos < e->start) { > const size_t off = e->start - e->pos; > > @@ -704,6 +707,51 @@ static void i915_error_vprintf(struct drm_i915_error_state_buf *e, > e->pos += len; > } > > +static void i915_error_vprintf(struct drm_i915_error_state_buf *e, > + const char *f, va_list args) > +{ > + unsigned len; > + > + if (!__i915_error_ok(e)) > + return; > + > + /* Seek the first printf which is hits start position */ > + if (e->pos < e->start) { > + len = vsnprintf(NULL, 0, f, args); > + if (!__i915_error_seek(e, len)) > + return; > + } > + > + len = vsnprintf(e->buf + e->bytes, e->size - e->bytes, f, args); > + if (len >= e->size - e->bytes) > + len = e->size - e->bytes - 1; > + > + __i915_error_advance(e, len); > +} > + > +static void i915_error_puts(struct drm_i915_error_state_buf *e, > + const char *str) > +{ > + unsigned len; > + > + if (!__i915_error_ok(e)) > + return; > + > + len = strlen(str); > + > + /* Seek the first printf which is hits start position */ > + if (e->pos < e->start) { > + if (!__i915_error_seek(e, len)) > + return; > + } > + > + if (len >= e->size - e->bytes) > + len = e->size - e->bytes - 1; > + memcpy(e->buf + e->bytes, str, len); > + > + __i915_error_advance(e, len); > +} > + > void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...) > { > va_list args; > @@ -714,6 +762,7 @@ void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...) > } > > #define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__) > +#define err_puts(e, s) i915_error_puts(e, s) > > static void print_error_buffers(struct drm_i915_error_state_buf *m, > const char *name, > @@ -723,26 +772,26 @@ static void print_error_buffers(struct drm_i915_error_state_buf *m, > err_printf(m, "%s [%d]:\n", name, count); > > while (count--) { > - err_printf(m, " %08x %8u %02x %02x %x %x%s%s%s%s%s%s%s", > + err_printf(m, " %08x %8u %02x %02x %x %x", > err->gtt_offset, > err->size, > err->read_domains, > err->write_domain, > - err->rseqno, err->wseqno, > - pin_flag(err->pinned), > - tiling_flag(err->tiling), > - dirty_flag(err->dirty), > - purgeable_flag(err->purgeable), > - err->ring != -1 ? " " : "", > - ring_str(err->ring), > - cache_level_str(err->cache_level)); > + err->rseqno, err->wseqno); > + err_puts(m, pin_flag(err->pinned)); > + err_puts(m, tiling_flag(err->tiling)); > + err_puts(m, dirty_flag(err->dirty)); > + err_puts(m, purgeable_flag(err->purgeable)); > + err_puts(m, err->ring != -1 ? " " : ""); > + err_puts(m, ring_str(err->ring)); > + err_puts(m, cache_level_str(err->cache_level)); > > if (err->name) > err_printf(m, " (name: %d)", err->name); > if (err->fence_reg != I915_FENCE_REG_NONE) > err_printf(m, " (fence: %d)", err->fence_reg); > > - err_printf(m, "\n"); > + err_puts(m, "\n"); > err++; > } > } > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
