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=-10.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 9B892C433E1 for ; Thu, 23 Jul 2020 11:08:34 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 65019206D7 for ; Thu, 23 Jul 2020 11:08:34 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="l7M0rI92"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="PiKQM3j9" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 65019206D7 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=7KPMrrHz/72QxPf4SJqwgv02GTRK9RkzUUGRYnlh2j8=; b=l7M0rI92rS9JXdB9qczZ1SkDt WMhPPJMWgZfIFOvFA3c0dJWxbwWNuIV/1pq1/f2rv0YacFIa6SgC6GV++fkl9EDw6vISzQMp8A6eL TTNPIiF9E+O20X5XWc+cdHWH2O/sXXQSSn6IfyK0Om29C4l5Zdh4gt1cn7f7XITxNejjpDcseCKks YBmjNG0RSc0gJsl74uL77GcYatvsbToay5lVKm2psu3MsQIhvO5HGi0/YZPKUT6CtQpBJcw7hmzLv 0AyPMjwzGRDYCcv2HOJ7lGtkNYysG/Lj91SzbWVB9exX2JRc+dJTjXYLGLM0KBPD1A9je87vaJcUn q4MBfC/TA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyZ48-0005Q1-Cc; Thu, 23 Jul 2020 11:07:08 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyZ44-0005Ol-5x for linux-arm-kernel@lists.infradead.org; Thu, 23 Jul 2020 11:07:05 +0000 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 76EF720709; Thu, 23 Jul 2020 11:07:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595502423; bh=dyrkPdy3Hqxe5cF7b/CUYGlzHQBomBDP3P9vcPrd/44=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PiKQM3j9HUFJ3SwcFTyxHmF6gF2ABeooWy9RHu55m/h+F62XbsXbp0lSrv/6vlywq OjmWKZdGF/ECqfaeyH0PvfljUbdm1hRwBJu3oxJrQWZ3RetNzFH+MmjN70QTIHVFMw dCoNuqRHxo3P8Eo9/lyjcDRibSRMB3GG98kDAiHQ= Date: Thu, 23 Jul 2020 13:07:07 +0200 From: Greg Kroah-Hartman To: Tingwei Zhang Subject: Re: [PATCH v4 06/20] coresight: add try_get_module() in coresight_grab_device() Message-ID: <20200723110707.GA1960107@kroah.com> References: <20200723042802.22511-1-tingwei@codeaurora.org> <20200723042802.22511-7-tingwei@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200723042802.22511-7-tingwei@codeaurora.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200723_070704_441517_C5674241 X-CRM114-Status: GOOD ( 22.47 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: tsoni@codeaurora.org, Sai Prakash Ranjan , Kim Phillips , Mathieu Poirier , Suzuki K Poulose , Alexander Shishkin , coresight@lists.linaro.org, Randy Dunlap , Mian Yousaf Kaukab , Russell King , Mao Jinlong , Leo Yan , linux-arm-kernel@lists.infradead.org, Mike Leach 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, Jul 23, 2020 at 12:27:48PM +0800, Tingwei Zhang wrote: > When coresight device is in an active session, driver module of > that device should not be removed. Use try_get_module() in > coresight_grab_device() to prevent module to be unloaded. Are you sure this works? Why is it needed at all? Why not just tear down the children properly when a module is removed so that you don't need this at all? > > Signed-off-by: Tingwei Zhang > --- > drivers/hwtracing/coresight/coresight.c | 27 +++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index b7151c5f81b1..17bc76ea86ae 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -640,7 +640,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id) > * don't appear on the trace path, they should be handled along with the > * the master device. > */ > -static void coresight_grab_device(struct coresight_device *csdev) > +static int coresight_grab_device(struct coresight_device *csdev) > { > int i; > > @@ -648,10 +648,25 @@ static void coresight_grab_device(struct coresight_device *csdev) > struct coresight_device *child; > > child = csdev->pdata->conns[i].child_dev; > - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) > + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { > + if (!try_module_get(child->dev.parent->driver->owner)) Why the child's parent? Why not the child itself? > + goto err; What about the error given to you here? Why throw that away? > pm_runtime_get_sync(child->dev.parent); > + } > } > + if (!try_module_get(csdev->dev.parent->driver->owner)) > + goto err; You don't reduce the child's parent's driver owner module reference here? Ugh, that's a horid sentence :( > pm_runtime_get_sync(csdev->dev.parent); > + return 0; > +err: > + for (i--; i >= 0; i--) { > + struct coresight_device *child; > + > + child = csdev->pdata->conns[i].child_dev; > + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) > + pm_runtime_put(child->dev.parent); > + } > + return -ENODEV; > } > > /* > @@ -663,12 +678,15 @@ static void coresight_drop_device(struct coresight_device *csdev) > int i; > > pm_runtime_put(csdev->dev.parent); > + module_put(csdev->dev.parent->driver->owner); > for (i = 0; i < csdev->pdata->nr_outport; i++) { > struct coresight_device *child; > > child = csdev->pdata->conns[i].child_dev; > - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) > + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { > pm_runtime_put(child->dev.parent); > + module_put(child->dev.parent->driver->owner); > + } > } > } > > @@ -721,7 +739,8 @@ static int _coresight_build_path(struct coresight_device *csdev, > if (!node) > return -ENOMEM; > > - coresight_grab_device(csdev); > + if (coresight_grab_device(csdev)) > + return -ENODEV; Why not return the error given to you? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel