public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Douglas Anderson <dianders@chromium.org>,
	David Airlie <airlied@linux.ie>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: Check for connector->state NULL in drm_atomic_add_affected_connectors
Date: Sun, 6 Mar 2016 19:30:15 +0200	[thread overview]
Message-ID: <20160306173015.GA10446@intel.com> (raw)
In-Reply-To: <20160306162951.GA14170@phenom.ffwll.local>

On Sun, Mar 06, 2016 at 05:29:51PM +0100, Daniel Vetter wrote:
> On Fri, Mar 04, 2016 at 03:43:53PM -0800, Douglas Anderson wrote:
> > On a system I'm doing development on I found a crash.  The crawl looked
> > like:
> > 
> >   PC is at drm_atomic_add_affected_connectors+0x98/0xe8
> >   ...
> >   drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344
> >   __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250
> >   restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80
> >   drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20
> >   rockchip_drm_lastclose from drm_lastclose+0x4c/0x104
> >   drm_lastclose from drm_release+0x424/0x47c
> >   drm_release from __fput+0xf8/0x1d4
> >   __fput from ____fput+0x18/0x1c
> >   ____fput from task_work_run+0xa8/0xbc
> >   task_work_run from do_exit+0x448/0x91c
> >   do_exit from do_group_exit+0x5c/0xcc
> >   do_group_exit from get_signal+0x4dc/0x57c
> >   get_signal from do_signal+0x9c/0x3b4
> >   do_signal from do_work_pending+0x60/0xb8
> >   do_work_pending from slow_work_pending+0xc/0x20
> > 
> > I found that I could fix the crash by checking connector->state against
> > NULL.  This isn't code I'm familiar with and I didn't dig too deep, so
> > I'd appreciate any opinions about whether this is a sane thing to do.
> > Note that I don't actually have graphics up on my system at the moment,
> > so perhaps this is all just a symptom of the strange state I'm in.
> > 
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> 
> This is a driver bug - under atomic the assumption is that there is
> _always_ a current software state. Most driver set up the initial
> "everything off" state in the ->reset functions.

Except if it's due to kzalloc() failing in the atomic helper reset
implementation. There's no way to propagate the error upwards, so
the current implementation just silently sets state=NULL and goes on
like nothing happened.

-- 
Ville Syrjälä
Intel OTC

  reply	other threads:[~2016-03-06 17:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-04 23:43 [PATCH] drm: Check for connector->state NULL in drm_atomic_add_affected_connectors Douglas Anderson
2016-03-06 16:29 ` Daniel Vetter
2016-03-06 17:30   ` Ville Syrjälä [this message]
2016-03-08  0:05   ` Doug Anderson
2016-03-08  0:09     ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160306173015.GA10446@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox