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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 723E4EB64D9 for ; Thu, 15 Jun 2023 14:07:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7No62aOR/eD4w7nyVSYVphaZF1WIbjE6jNHGZUEWUeg=; b=yaqa09li2G89P3 U5sSeUxXRLtt5fCLmLNYGCL0grN2oxLgxb5uVok6NRKdNu37nFfunj5iZcxtArnzTRj3PYciPq3tF +2B/uNmhOaYjsqSSfYCGWHt6ctRVPspmxDE96nyfCcbZ5OQMvmYeh+Fgu+G+eWub+PYzMZ/7B6KKO mAdY4CuLNJay7NCY3Q1EluluK/PeaHIFS+moDUMTGu2e5xuyZ8aP+5EggsyedYCCodY84Q+ELpD4e gNlUYQJrp2rhG7lgbQ7Y2LvYSqQJvVrynyZFnFjDdwfAdWVkS6HyZDfr9ai1wzYRPR9JGdF9PhGCB w+LvVCJVct8DODUL3lIg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q9ncV-00F5Wo-37; Thu, 15 Jun 2023 14:06:39 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q9ncT-00F5W5-0H for linux-arm-kernel@lists.infradead.org; Thu, 15 Jun 2023 14:06:38 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 99E0F62B57; Thu, 15 Jun 2023 14:06:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 85F76C433C8; Thu, 15 Jun 2023 14:06:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1686837996; bh=2HWxn6SgFtrNnxm9GSS8Rtq52vTNyCTVVqs90CZgSGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KpR36luzMbFukaoYSxIo/2ySvXZ9EI94ww6JPoZMp9yWn31FB5/1Jc+/1VCbSY/ed xm7JCcRRg7tG5aCgWpZ765U8UWEq/1DZFZF0WW6EFcuYvS4QejYWGHzrK8LmHnV9g1 JMqUL53yXK4Pw9ebkLssTsQF9JW28YovjMw72+0E= Date: Thu, 15 Jun 2023 16:06:33 +0200 From: Greg Kroah-Hartman To: Souradeep Chowdhury Cc: Andy Gross , Konrad Dybcio , Krzysztof Kozlowski , Bjorn Andersson , Rob Herring , Alex Elder , Arnd Bergmann , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, Sibi Sankar , Rajendra Nayak Subject: Re: [PATCH V23 2/3] misc: dcc: Add driver support for Data Capture and Compare unit(DCC) Message-ID: <2023061548-subtly-cackle-8be2@gregkh> References: <2259ab0348282349e88905ea99bcb4aa815d941f.1683265984.git.quic_schowdhu@quicinc.com> <2023061542-reformed-unholy-10a3@gregkh> <2023061515-unbuckled-consonant-e207@gregkh> <5d9ab90f-4fc3-26c6-141e-e9388ac2f0cf@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5d9ab90f-4fc3-26c6-141e-e9388ac2f0cf@quicinc.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230615_070637_171437_A6CE4069 X-CRM114-Status: GOOD ( 24.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jun 15, 2023 at 07:17:34PM +0530, Souradeep Chowdhury wrote: > > > > > +static ssize_t ready_read(struct file *filp, char __user *userbuf, > > > > > + size_t count, loff_t *ppos) > > > > > +{ > > > > > + int ret = 0; > > > > > + char *buf; > > > > > + struct dcc_drvdata *drvdata = filp->private_data; > > > > > + > > > > > + mutex_lock(&drvdata->mutex); > > > > > + > > > > > + if (!is_dcc_enabled(drvdata)) { > > > > > + ret = -EINVAL; > > > > > + goto out_unlock; > > > > > + } > > > > > + > > > > > + if (!FIELD_GET(BIT(1), readl(drvdata->base + dcc_status(drvdata->mem_map_ver)))) > > > > > + buf = "Y\n"; > > > > > + else > > > > > + buf = "N\n"; > > > > > +out_unlock: > > > > > + mutex_unlock(&drvdata->mutex); > > > > > + > > > > > + if (ret < 0) > > > > > + return -EINVAL; > > > > > + else > > > > > > > > You do the "lock, get a value, unlock, do something with the value" > > > > thing a bunch, but what prevents the value from changing after the lock > > > > happens? So why is the lock needed at all? > > > > > > The lock is used to prevent concurrent accesses of the drv_data when > > > scripts are being run from userspace. > > > > How would that matter? The state can change instantly after the lock is > > given up, and then the returned value is now incorrect. So no need for > > a lock at all as you really aren't "protecting" anything, or am I > > missing something else? > > This lock is needed to protect the access to the global instance of drv_data > structure instantiated at probe time within each individual callbacks of > debugfs. What exactly are you "protecting" here that could change in a way that cause a problem? You aren't returning a value that is ever guaranteed to be "correct" except that it happened sometime in the past, it might be right anymore. thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel