From patchwork Fri Jul 19 15:23:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 11050263 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 46B9014DB for ; Fri, 19 Jul 2019 15:23:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3542228488 for ; Fri, 19 Jul 2019 15:23:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 28E0D28928; Fri, 19 Jul 2019 15:23:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id BDB2528488 for ; Fri, 19 Jul 2019 15:23:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2415C6E84D; Fri, 19 Jul 2019 15:23:27 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by gabe.freedesktop.org (Postfix) with ESMTPS id 29C8C6E84E for ; Fri, 19 Jul 2019 15:23:24 +0000 (UTC) Received: by mail-ed1-x542.google.com with SMTP id i11so34829816edq.0 for ; Fri, 19 Jul 2019 08:23:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=xFHOTLwp+b/dZE4sngfYD0eMlUdDzWSkwvsokg6afgA=; b=ZOh+E+6+FldIFNbgnBNBlq6X4EkneFsoOPwmgxnij47x1mV2vBKyqz00dhWfVR4aXT tXJtw1O6ibQWGxePuNQJ+xHEPWAvwFU+k8g8xHSpdiRD18yKTgVrvFztJnkl3oF9ZiUl lGRgfeCC5U7zphg5q5C2POFC3yP4aI8rQLWOU4HQdN07nMeB0sF8U/x25AQ8Ofgqo7Oj iwsaJ3Omk9GHR7Ig+m3tvqDPBAenVwvwg+1tG/bS6KEawWmmOPSc08csMMKqlLRlq2RF 9hIXp2Ed5U4Y4f1S/DQFP7UeTUS/AkUGr7u8+qs5nFAr6iFCrtEL/IwjZD0ncyz5naDn Df1g== X-Gm-Message-State: APjAAAWea4hopiianW4etv2tysV6ALqEWPCYeztGI07tMEyEbe6vW2Pa WVRUPS+7EuaflX2KY6M1D7kSWcXM X-Google-Smtp-Source: APXvYqyNOC9FzBSgRhiu0c1Eppme2UT4kllyHI4EpR3J46DXhp//MuZX5G03DisxbZklt3UtnbLi/A== X-Received: by 2002:a17:906:cce9:: with SMTP id ot41mr12688698ejb.196.1563549802546; Fri, 19 Jul 2019 08:23:22 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id t2sm8882538eda.95.2019.07.19.08.23.21 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 19 Jul 2019 08:23:21 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 3/3] drm/vkms: Reduce critical section in vblank_simulate Date: Fri, 19 Jul 2019 17:23:14 +0200 Message-Id: <20190719152314.7706-3-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190719152314.7706-1-daniel.vetter@ffwll.ch> References: <20190719152314.7706-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xFHOTLwp+b/dZE4sngfYD0eMlUdDzWSkwvsokg6afgA=; b=WsyOLyahh9jkWf8mBub4V5UlJN5P4orW8IeSpSX+i5yGPUzTnoZ0XEdRm/ONV1D6s1 GFrl1sz67IK/S/hq2UWLZbWxNLauZ4E26Ekt6A2k3fFDLRJWwtcaDAFC6dwPdxTeo7Hb JVwS3xntMOzTYCgBbDW/8b+bU2iVydZagiglg= X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Haneen Mohammed , Rodrigo Siqueira , Daniel Vetter , Intel Graphics Development , Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP We can reduce the critical section in vkms_vblank_simulate under output->lock quite a lot: - hrtimer_forward_now just needs to be ordered correctly wrt drm_crtc_handle_vblank. We already access the hrtimer timestamp without locks. While auditing that I noticed that we don't correctly annotate the read there, so sprinkle a READ_ONCE to make sure the compiler doesn't do anything foolish. - drm_crtc_handle_vblank must stay under the lock to avoid races with drm_crtc_arm_vblank_event. - The access to vkms_ouptut->crc_state also must stay under the lock. - next problem is making sure the output->state structure doesn't get freed too early. First we rely on a given hrtimer being serialized: If we call drm_crtc_handle_vblank, then we are guaranteed that the previous call to vkms_vblank_simulate has completed. The other side of the coin is that the atomic updates waits for the vblank to happen before it releases the old state. Both taken together means that by the time the atomic update releases the old state, the hrtimer won't access it anymore (it might be accessing the new state at the same time, but that's ok). - state is invariant, except the few fields separate protected by state->crc_lock. So no need to hold the lock for that. - finally the queue_work. We need to make sure there's no races with the flush_work, i.e. when we call flush_work we need to guarantee that the hrtimer can't requeue the work again. This is guaranteed by the same vblank/hrtimer ordering guarantees like the reasoning above why state won't be freed too early: flush_work on the old state is called after wait_for_flip_done in the atomic commit code. Therefore we can also move everything after the output->crc_state out of the critical section. Motivated by suggestions from Rodrigo. Signed-off-by: Daniel Vetter Cc: Rodrigo Siqueira Cc: Haneen Mohammed Cc: Daniel Vetter Reviewed-by: Rodrigo Siqueira --- drivers/gpu/drm/vkms/vkms_crtc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 927dafaebc76..74f703b8d22a 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -16,17 +16,18 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) u64 ret_overrun; bool ret; - spin_lock(&output->lock); - ret_overrun = hrtimer_forward_now(&output->vblank_hrtimer, output->period_ns); WARN_ON(ret_overrun != 1); + spin_lock(&output->lock); ret = drm_crtc_handle_vblank(crtc); if (!ret) DRM_ERROR("vkms failure on handling vblank"); state = output->composer_state; + spin_unlock(&output->lock); + if (state && output->composer_enabled) { u64 frame = drm_crtc_accurate_vblank_count(crtc); @@ -48,8 +49,6 @@ static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer) DRM_DEBUG_DRIVER("Composer worker already queued\n"); } - spin_unlock(&output->lock); - return HRTIMER_RESTART; } @@ -85,7 +84,7 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, struct vkms_output *output = &vkmsdev->output; struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; - *vblank_time = output->vblank_hrtimer.node.expires; + *vblank_time = READ_ONCE(output->vblank_hrtimer.node.expires); if (WARN_ON(*vblank_time == vblank->time)) return true;