From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Hellstrom <thellstrom@vmware.com>
Subject: Re: [BUG] completely bonkers use of set_need_resched +
	VM_FAULT_NOPAGE
Date: Thu, 12 Sep 2013 22:48:15 +0200
Message-ID: <5232288F.4070904@vmware.com>
References: <20130912150645.GZ31370@twins.programming.kicks-ass.net>
	<CAKMK7uFF2GkYapa2_DLv4WYAd0t+POaT+Vy9HG=HQhzMmtH9nA@mail.gmail.com>
	<20130912154329.GB31370@twins.programming.kicks-ass.net>
	<CAKMK7uFTYB-E5ksVNEBqGJusJ5TKJv5X4hiPv1mno9tJKuUAdA@mail.gmail.com>
	<20130912162210.GE31370@twins.programming.kicks-ass.net>
	<CAKMK7uGsgYOtKQrCoq26RV9a_8na14sbpanUxM2_3EBwz-3vVw@mail.gmail.com>
	<alpine.DEB.2.02.1309122157300.4089@ionos.tec.linutronix.de>
	<CAKMK7uHJansZzKxrr8p0c2X5pEeb-BSxLa6=tnsDmwo-k__Xcg@mail.gmail.com>
	<alpine.DEB.2.02.1309122219370.4089@ionos.tec.linutronix.de>
	<CAKMK7uHqufs64=pD=o--958Ns+NAsrwUTNpcd2_WGQnoJRnfOQ@mail.gmail.com>
	<alpine.DEB.2.02.1309122238430.4089@ionos.tec.linutronix.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
In-Reply-To: <alpine.DEB.2.02.1309122238430.4089@ionos.tec.linutronix.de>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>, Daniel Vetter <daniel.vetter@ffwll.ch>, intel-gfx <intel-gfx@lists.freedesktop.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, dri-devel <dri-devel@lists.freedesktop.org>, Dave Airlie <airlied@linux.ie>, Maarten Lankhorst <maarten.lankhorst@canonical.com>, Ingo Molnar <mingo@kernel.org>
List-Id: intel-gfx@lists.freedesktop.org

On 09/12/2013 10:39 PM, Thomas Gleixner wrote:
> On Thu, 12 Sep 2013, Daniel Vetter wrote:
>
>> On Thu, Sep 12, 2013 at 10:20 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>>> I think for ttm drivers it's just execbuf being exploitable. But on
>>>> drm/i915 we've
>>>> had the same issue with the pwrite/pread ioctls, so a simple
>>>> glBufferData(glMap) kind of recursion from gl clients blew the kernel
>>>> to pieces ...
>>> And the only answer you folks came up with is set_need_resched() and
>>> yield()? Oh well....
>> The yield was for a different lifelock, and that one is also fixed by
>> now. The fault handler deadlock was fixed in the usual "drop locks and
>> jump into slowpath" fasion, at least in drm/i915.
> So we can remove that whole yield/set_need_resched() mess completely ?
>
> Thanks,
>
> 	tglx
No.

The while(trylock) is there to address a potential locking inversion 
deadlock. If the trylock fails, the code returns to user-space which 
retries the fault. This code needs to stay until we can come up with 
either a way to drop the mmap_sem and sleep before returning to 
user-space, or a bunch of code is fixed with a different locking order.

The set_need_resched() can (and should according to Peter) go.

/Thomas