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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7467EEB64DA for ; Thu, 15 Jun 2023 14:06:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344757AbjFOOGi (ORCPT ); Thu, 15 Jun 2023 10:06:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39266 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344716AbjFOOGh (ORCPT ); Thu, 15 Jun 2023 10:06:37 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0605B1BE8; Thu, 15 Jun 2023 07:06:37 -0700 (PDT) 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 963D4629A6; 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-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5d9ab90f-4fc3-26c6-141e-e9388ac2f0cf@quicinc.com> Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.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