linux-kernel - Re: [ 109/173] drm/i915: Close race between processing unpin task and queueing the flip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20130213001812.GV13292@decadent.org.uk>
Date:	Wed, 13 Feb 2013 00:18:12 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Herton Ronaldo Krzesinski <herton.krzesinski@...onical.com>
Cc:	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
	Chris Wilson <chris@...is-wilson.co.uk>,
	Daniel Vetter <daniel.vetter@...ll.ch>
Subject: Re: [ 109/173] drm/i915: Close race between processing unpin task
 and queueing the flip

On Tue, Feb 12, 2013 at 09:02:45PM -0200, Herton Ronaldo Krzesinski wrote:
> On Fri, Dec 28, 2012 at 08:05:19PM +0100, Ben Hutchings wrote:
> > 3.2-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Chris Wilson <chris@...is-wilson.co.uk>
> > 
> > commit e7d841ca03b7ab668620045cd7b428eda9f41601 upstream.
> > 
> [...]
> > @@ -7157,6 +7176,10 @@ static int intel_gen4_queue_flip(struct
> >  	pf = 0;
> >  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
> >  	OUT_RING(pf | pipesrc);
> > +
> > +	intel_mark_page_flip_active(intel_crtc);
> > +
> > +	intel_mark_page_flip_active(intel_crtc);
> >  	ADVANCE_LP_RING();
> >  	return 0;
> >  
> 
> There is a problem with this patch on 3.2. First we have
> intel_mark_page_flip_active duplicated on intel_gen4_queue_flip here
> (harmless), but one missing call for it in intel_gen6_queue_flip

Yes, Julien Cristau also pointed this out.  I'm not sure how this
happened.

It does seem that quilt (presumably calling GNU patch?) can sometimes
apply two hunks that overlap, and maybe that's part of the explanation
for this.  In 3.2.38 there was one patch where a blank line was the
last line of context in one hunk and also the first line of context
for the next hunk.  (Upstream, there was an extra statement in
between, but this was not present in 3.2.y.)  quilt applied the patch
without complaint (with --fuzz=0) but git am rejected it.

> Reached to this after reproducing which is probably the same
> problem people are reporting here: http://bugs.launchpad.net/bugs/1119809
> A bisect pointed to this change, and the following diff fixed the issue
> on the i915 based machine I used to reproduce:
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4da3c7e..ce23961 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7155,8 +7155,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
>  	OUT_RING(pf | pipesrc);
>  
>  	intel_mark_page_flip_active(intel_crtc);
> -
> -	intel_mark_page_flip_active(intel_crtc);
>  	ADVANCE_LP_RING();
>  	return 0;
>  
> @@ -7192,6 +7190,8 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
>  	pf = I915_READ(PF_CTL(intel_crtc->pipe)) & PF_ENABLE;
>  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
>  	OUT_RING(pf | pipesrc);
> +
> +	intel_mark_page_flip_active(intel_crtc);
>  	ADVANCE_LP_RING();
>  	return 0;
>  
 
Thanks, but I have the same fix in the patch queue already.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/