From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2] drm: do not sleep on vblank while holding a mutex Date: Sat, 29 Oct 2011 12:00:20 +0200 Message-ID: <20111029100020.GC2932@phenom.ffwll.local> References: <1319838143-2318-1-git-send-email-ihadzic@research.bell-labs.com> <20111029005819.GF2919@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wy0-f177.google.com (mail-wy0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 0C0D39E9D3 for ; Sat, 29 Oct 2011 02:59:26 -0700 (PDT) Received: by wyg8 with SMTP id 8so5756693wyg.36 for ; Sat, 29 Oct 2011 02:59:26 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Ilija Hadzic Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Fri, Oct 28, 2011 at 09:44:54PM -0500, Ilija Hadzic wrote: > Now suppose that the drm_wait_vblank is enteret in the context of > two different PIDs and suppose that there are no locks. Let's say > that the first process wants to wait on vblank N and the second > wants to wait on vblank N+1. First process can reach the point just > before it wants to write to last_vblank_wait and lose the processor > there (so it has N in vblank.request (on its stack) calculated, but > it has not written it into last_vblank_wait yet (because it lost the > CPU right there). Then the second process gets the CPU, and executes > and let's say that it goes all the way through so it writes N+1 into > last_vblank_wait and then schedules away (it called DRM_WAIT_ON). > Now the first process gets the CPU where it left off and overwrites > N+1 in last_vblank_wait with N, which is now stale. Ok, and here's why your locking (or any locking that drops the lock before calling schedule) won't work: Dropping the lock allows more than one process waiting for a vblank on the same crtc (the hole point of this exerices). So let's say N processes are waiting for vblank on crtc A, but we only have one storage place for last_vblank_wait. So by necessity, it's gonna be stale for N-1 processes. And because the scheduler is free to take away the cpu between the unlock and the schedule, you can freely pick for which process it's gonna be the right one, there's zero ordering guarantees possible. By throwing in a modeset call that disables vblanks it's pretty easy to come up with a process that waits for vblank x+1 (when x is the last vblank before the vblank counter got stopped before the modeset) and last_vblank_wait == x. Which is why I think adding decent tracepoints would be much better, but that's a different patch. [snip] Agreed on not locking the debug file and postponing other cleanups, that's all material for different patches. Yours, Daniel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48