From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB228C433E1 for ; Thu, 21 May 2020 00:10:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C26062075F for ; Thu, 21 May 2020 00:10:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726748AbgEUAKS (ORCPT ); Wed, 20 May 2020 20:10:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34330 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726619AbgEUAKS (ORCPT ); Wed, 20 May 2020 20:10:18 -0400 Received: from Galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8AD09C061A0E; Wed, 20 May 2020 17:10:18 -0700 (PDT) Received: from [5.158.153.53] (helo=debian-buster-darwi.lab.linutronix.de) by Galois.linutronix.de with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1jbYmC-0006mO-Af; Thu, 21 May 2020 02:09:32 +0200 Date: Thu, 21 May 2020 02:09:30 +0200 From: "Ahmed S. Darwish" To: Christian =?iso-8859-1?Q?K=F6nig?= Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , "David (ChunMing) Zhou" , amd-gfx@lists.freedesktop.org, "Paul E. McKenney" , David Airlie , "Sebastian A. Siewior" , LKML , Steven Rostedt , Christian =?iso-8859-1?Q?K=F6nig?= , dri-devel@lists.freedesktop.org, Daniel Vetter , Alex Deucher , Felix Kuehling , Thomas Gleixner , Sumit Semwal , linux-media@vger.kernel.org Subject: Re: [PATCH v1 13/25] dma-buf: Use sequence counter with associated wound/wait mutex Message-ID: <20200521000930.GA359643@debian-buster-darwi.lab.linutronix.de> References: <20200519214547.352050-1-a.darwish@linutronix.de> <20200519214547.352050-14-a.darwish@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Wed, May 20, 2020, Christian König wrote: > Am 19.05.20 um 23:45 schrieb Ahmed S. Darwish: > > A sequence counter write side critical section must be protected by some > > form of locking to serialize writers. If the serialization primitive is > > not disabling preemption implicitly, preemption has to be explicitly > > disabled before entering the sequence counter write side critical > > section. > > > > The dma-buf reservation subsystem uses plain sequence counters to manage > > updates to reservations. Writer serialization is accomplished through a > > wound/wait mutex. > > > > Acquiring a wound/wait mutex does not disable preemption, so this needs > > to be done manually before and after the write side critical section. > > > > Use the newly-added seqcount_ww_mutex_t instead: > > > > - It associates the ww_mutex with the sequence count, which enables > > lockdep to validate that the write side critical section is properly > > serialized. > > > > - It removes the need to explicitly add preempt_disable/enable() > > around the write side critical section because the write_begin/end() > > functions for this new data type automatically do this. > > > > If lockdep is disabled this ww_mutex lock association is compiled out > > and has neither storage size nor runtime overhead. > > Mhm, is the dma_resv object the only user of this new seqcount_ww_mutex > variant ? > > If yes we are trying to get rid of this sequence counter for quite some > time, so I would rather invest the additional time to finish this. > In this patch series, each extra "seqcount with associated lock" data type costs us, exactly: - 1 typedef definition, seqcount_ww_mutex_t - 1 static initializer, SEQCNT_WW_MUTEX_ZERO() - 1 runtime initializer, seqcount_ww_mutex_init() Definitions for the typedef and the 2 initializers above are template-code one liners. The logic which automatically disables preemption upon entering a seqcount_ww_mutex_t write side critical section is also already shared with seqcount_mutex_t and any future, preemptible, associated lock. So, yes, dma-resv is the only user of seqcount_ww_mutex. But even in that case, given the one liner template code nature of seqcount_ww_mutex_t logic, it does not make sense to block the dma_resv and amdgpu change until at some point in the future the sequence counter is completely removed. **If and when** the sequence counter gets removed, please just remove the seqcount_ww_mutex_t data type with it. It will be extremely simple. > Regards, > Christian. > Thanks, -- Ahmed S. Darwish Linutronix GmbH