| Message ID | 1386678241-18228-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 10, 2013 at 01:24:01PM +0100, Daniel Vetter wrote: > The update is horribly racy since it doesn't protect at all against > concurrent closing of the master fd. And it can't really since that > requires us to grab a mutex. > > Instead of jumping through hoops and offloading this to a worker > thread just block this bit of code for the modesetting driver. > > Reported-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > Cc: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > Cc: stable@vger.kernel.org > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> We should also not be calling update_dri1_breadcrumb unless we have a USER_INTERRUPT. The other updates to the breadcrumb hws index do not appear to be serialised by anything other than polling. -Chris
On Tue, Dec 10, 2013 at 12:44:12PM +0000, Chris Wilson wrote: > On Tue, Dec 10, 2013 at 01:24:01PM +0100, Daniel Vetter wrote: > > The update is horribly racy since it doesn't protect at all against > > concurrent closing of the master fd. And it can't really since that > > requires us to grab a mutex. > > > > Instead of jumping through hoops and offloading this to a worker > > thread just block this bit of code for the modesetting driver. > > > > Reported-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > > Cc: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > We should also not be calling update_dri1_breadcrumb unless we have a > USER_INTERRUPT. The other updates to the breadcrumb hws index do not > appear to be serialised by anything other than polling. Well, this is what's been there before and I don't really want to touch dri1/ums code at all. So I think we'll just keep this here until we'll all put it on the big pyre ;-) -Daniel
On Tue, Dec 10, 2013 at 02:48:23PM +0100, Daniel Vetter wrote: > On Tue, Dec 10, 2013 at 12:44:12PM +0000, Chris Wilson wrote: > > On Tue, Dec 10, 2013 at 01:24:01PM +0100, Daniel Vetter wrote: > > > The update is horribly racy since it doesn't protect at all against > > > concurrent closing of the master fd. And it can't really since that > > > requires us to grab a mutex. > > > > > > Instead of jumping through hoops and offloading this to a worker > > > thread just block this bit of code for the modesetting driver. > > > > > > Reported-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > > > Cc: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > We should also not be calling update_dri1_breadcrumb unless we have a > > USER_INTERRUPT. The other updates to the breadcrumb hws index do not > > appear to be serialised by anything other than polling. > > Well, this is what's been there before and I don't really want to touch > dri1/ums code at all. So I think we'll just keep this here until we'll all > put it on the big pyre ;-) Chris and I discusssed the commit message a bit on irc and now the hack also spurts a code comment. Merged to -fixes for now, but I'd still like to get a tested by from Eugene before sending off the pull request. -Daniel
On 12/11/2013 03:06 PM, Daniel Vetter wrote: > On Tue, Dec 10, 2013 at 02:48:23PM +0100, Daniel Vetter wrote: >> On Tue, Dec 10, 2013 at 12:44:12PM +0000, Chris Wilson wrote: >>> On Tue, Dec 10, 2013 at 01:24:01PM +0100, Daniel Vetter wrote: >>>> The update is horribly racy since it doesn't protect at all against >>>> concurrent closing of the master fd. And it can't really since that >>>> requires us to grab a mutex. >>>> >>>> Instead of jumping through hoops and offloading this to a worker >>>> thread just block this bit of code for the modesetting driver. >>>> >>>> Reported-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> >>>> Cc: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> >>> >>> We should also not be calling update_dri1_breadcrumb unless we have a >>> USER_INTERRUPT. The other updates to the breadcrumb hws index do not >>> appear to be serialised by anything other than polling. >> >> Well, this is what's been there before and I don't really want to touch >> dri1/ums code at all. So I think we'll just keep this here until we'll all >> put it on the big pyre ;-) > > Chris and I discusssed the commit message a bit on irc and now the hack > also spurts a code comment. Merged to -fixes for now, but I'd still like > to get a tested by from Eugene before sending off the pull request. > -Daniel > Built 3.10.23 with the fix, tested today. The problem has not shown up so far. Tested-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru>
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 197db0993a24..0a42e2f7bcf8 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -83,6 +83,9 @@ void i915_update_dri1_breadcrumb(struct drm_device *dev) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_master_private *master_priv; + if (drm_core_check_feature(dev, DRIVER_MODESET)) + return; + if (dev->primary->master) { master_priv = dev->primary->master->driver_priv; if (master_priv->sarea_priv)
The update is horribly racy since it doesn't protect at all against concurrent closing of the master fd. And it can't really since that requires us to grab a mutex. Instead of jumping through hoops and offloading this to a worker thread just block this bit of code for the modesetting driver. Reported-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> Cc: Eugene Shatokhin <eugene.shatokhin@rosalab.ru> Cc: stable@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ 1 file changed, 3 insertions(+)