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 C875AC433F5 for ; Mon, 9 May 2022 16:56:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239297AbiEIRAb (ORCPT ); Mon, 9 May 2022 13:00:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42170 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239330AbiEIRA2 (ORCPT ); Mon, 9 May 2022 13:00:28 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2876654000; Mon, 9 May 2022 09:56:28 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id B888E614F5; Mon, 9 May 2022 16:56:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BFCA4C385B1; Mon, 9 May 2022 16:56:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652115387; bh=NT/NSm+ixv4NTnm+bzYUbh8NWftsy0GkupiwFftUrCw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=EtvoF8IpfrUKSdvwW8V1qeHxhWfkW598IrjNsAqteSHaADCcEdw4x8KXDr3TbpGco +yggH/xodb4xZyPEnEeIc8mWLqDjS4iXNJXSCuBQpOQ/fSanq8ns6RptBh90cQlffa ZydWP2ygHERY2FiBF7EYP2PUX0oJed77PI+PtGtAug8JPiOmVIAJvSdJSVF0l2WgT3 wW5f+Mu+92KTr/DOaR7Nqi5mZ7fd/NZ7bLFFprVwVSRAaFOoxIjPeDgJTM5EOXSyV1 crJcVua2rcdRuwnbPejGtAvrTv7VAS/uq+3pJDH8Q5DUtKss9HWmbgKe7fxadeJAws EJ34J+Ehy52jg== Date: Mon, 9 May 2022 18:56:20 +0200 From: Mauro Carvalho Chehab To: Andi Shyti Cc: Luis Chamberlain , alsa-devel@alsa-project.org, mauro.chehab@linux.intel.com, David Airlie , Greg KH , intel-gfx@lists.freedesktop.org, Lucas De Marchi , Takashi Iwai , dri-devel@lists.freedesktop.org, Jaroslav Kysela , Kai Vehmanen , linux-modules@vger.kernel.org, Dan Williams , linux-kernel@vger.kernel.org, Pierre-Louis Bossart Subject: Re: [Intel-gfx] [PATCH v5 1/2] module: update dependencies at try_module_get() Message-ID: <20220509185620.05567716@coco.lan> In-Reply-To: References: X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: Em Thu, 5 May 2022 23:35:29 +0200 Andi Shyti escreveu: > Hi Mauro, > > [...] > > > +static int ref_module_dependency(struct module *mod, struct module *this) > > +{ > > + int ret; > > + > > + if (!this || !this->name) > > + return -EINVAL; > > + > > + if (mod == this) > > + return 0; > > + > > + mutex_lock(&module_mutex); > > + > > + ret = ref_module(this, mod); > > + > > +#ifdef CONFIG_MODULE_UNLOAD > > + if (ret) > > + goto ret; > > + > > + ret = sysfs_create_link(mod->holders_dir, > > + &this->mkobj.kobj, this->name); > > +#endif > > + > > +ret: > > + mutex_unlock(&module_mutex); > > + return ret; > > +} > > + > > /* Clear the unload stuff of the module. */ > > static void module_unload_free(struct module *mod) > > { > > @@ -841,24 +886,16 @@ void __module_get(struct module *module) > > } > > EXPORT_SYMBOL(__module_get); > > > > -bool try_module_get(struct module *module) > > +bool try_module_get_owner(struct module *module, struct module *this) > > { > > - bool ret = true; > > + int ret = __try_module_get(module); > > > > - if (module) { > > - preempt_disable(); > > - /* Note: here, we can fail to get a reference */ > > - if (likely(module_is_live(module) && > > - atomic_inc_not_zero(&module->refcnt) != 0)) > > - trace_module_get(module, _RET_IP_); > > - else > > - ret = false; > > + if (ret) > > + ref_module_dependency(module, this); > > do we care about the return value here? I don't think it should care about the return value, as a failure to create a sysfs node for the holder or to add it to the holders list is not fatal: modules can still continue working without that. Also, I opted to be conservative here: currently, not creating these doesn't cause try_module_get() to fail. I'm not sure what would be the impact if this starts to fail. So, right now, I'm opting to just ignore the return value. Perhaps in the future this could a warning (similarly to what sysfs create link does). Regards, Mauro > > Andi > > > > > - preempt_enable(); > > - } > > return ret; > > } > > -EXPORT_SYMBOL(try_module_get); > > +EXPORT_SYMBOL(try_module_get_owner); > > > > void module_put(struct module *module) > > { > > -- > > 2.35.1 Thanks, Mauro 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 56BFEC433F5 for ; Mon, 9 May 2022 16:56:36 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0F21810F40F; Mon, 9 May 2022 16:56:33 +0000 (UTC) Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by gabe.freedesktop.org (Postfix) with ESMTPS id 67BF610EFA4; Mon, 9 May 2022 16:56:31 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 62A69B8180F; Mon, 9 May 2022 16:56:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BFCA4C385B1; Mon, 9 May 2022 16:56:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652115387; bh=NT/NSm+ixv4NTnm+bzYUbh8NWftsy0GkupiwFftUrCw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=EtvoF8IpfrUKSdvwW8V1qeHxhWfkW598IrjNsAqteSHaADCcEdw4x8KXDr3TbpGco +yggH/xodb4xZyPEnEeIc8mWLqDjS4iXNJXSCuBQpOQ/fSanq8ns6RptBh90cQlffa ZydWP2ygHERY2FiBF7EYP2PUX0oJed77PI+PtGtAug8JPiOmVIAJvSdJSVF0l2WgT3 wW5f+Mu+92KTr/DOaR7Nqi5mZ7fd/NZ7bLFFprVwVSRAaFOoxIjPeDgJTM5EOXSyV1 crJcVua2rcdRuwnbPejGtAvrTv7VAS/uq+3pJDH8Q5DUtKss9HWmbgKe7fxadeJAws EJ34J+Ehy52jg== Date: Mon, 9 May 2022 18:56:20 +0200 From: Mauro Carvalho Chehab To: Andi Shyti Subject: Re: [Intel-gfx] [PATCH v5 1/2] module: update dependencies at try_module_get() Message-ID: <20220509185620.05567716@coco.lan> In-Reply-To: References: X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mauro.chehab@linux.intel.com, linux-kernel@vger.kernel.org, David Airlie , Greg KH , intel-gfx@lists.freedesktop.org, Lucas De Marchi , alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org, Takashi Iwai , Kai Vehmanen , Luis Chamberlain , Dan Williams , Jaroslav Kysela , linux-modules@vger.kernel.org, Pierre-Louis Bossart Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Em Thu, 5 May 2022 23:35:29 +0200 Andi Shyti escreveu: > Hi Mauro, > > [...] > > > +static int ref_module_dependency(struct module *mod, struct module *this) > > +{ > > + int ret; > > + > > + if (!this || !this->name) > > + return -EINVAL; > > + > > + if (mod == this) > > + return 0; > > + > > + mutex_lock(&module_mutex); > > + > > + ret = ref_module(this, mod); > > + > > +#ifdef CONFIG_MODULE_UNLOAD > > + if (ret) > > + goto ret; > > + > > + ret = sysfs_create_link(mod->holders_dir, > > + &this->mkobj.kobj, this->name); > > +#endif > > + > > +ret: > > + mutex_unlock(&module_mutex); > > + return ret; > > +} > > + > > /* Clear the unload stuff of the module. */ > > static void module_unload_free(struct module *mod) > > { > > @@ -841,24 +886,16 @@ void __module_get(struct module *module) > > } > > EXPORT_SYMBOL(__module_get); > > > > -bool try_module_get(struct module *module) > > +bool try_module_get_owner(struct module *module, struct module *this) > > { > > - bool ret = true; > > + int ret = __try_module_get(module); > > > > - if (module) { > > - preempt_disable(); > > - /* Note: here, we can fail to get a reference */ > > - if (likely(module_is_live(module) && > > - atomic_inc_not_zero(&module->refcnt) != 0)) > > - trace_module_get(module, _RET_IP_); > > - else > > - ret = false; > > + if (ret) > > + ref_module_dependency(module, this); > > do we care about the return value here? I don't think it should care about the return value, as a failure to create a sysfs node for the holder or to add it to the holders list is not fatal: modules can still continue working without that. Also, I opted to be conservative here: currently, not creating these doesn't cause try_module_get() to fail. I'm not sure what would be the impact if this starts to fail. So, right now, I'm opting to just ignore the return value. Perhaps in the future this could a warning (similarly to what sysfs create link does). Regards, Mauro > > Andi > > > > > - preempt_enable(); > > - } > > return ret; > > } > > -EXPORT_SYMBOL(try_module_get); > > +EXPORT_SYMBOL(try_module_get_owner); > > > > void module_put(struct module *module) > > { > > -- > > 2.35.1 Thanks, Mauro 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 alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (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 DB1E4C433F5 for ; Mon, 9 May 2022 16:57:32 +0000 (UTC) Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id D9AAF18BC; Mon, 9 May 2022 18:56:39 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz D9AAF18BC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1652115449; bh=NT/NSm+ixv4NTnm+bzYUbh8NWftsy0GkupiwFftUrCw=; h=Date:From:To:Subject:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=qNqHJ17ihyXQSwft0iC33JG72zSMh1/u/4lzmUwvj9AP2PH1WSAE8jZvbtwaCMfLx fvYKOdu6pnGvfYUy0FAkl7CI0KTdXWpQUcGAdTxbn/qdxqBTUd8VQhVy86IvFZwoCJ UkggRAjeze0blO7sf6Xgisnl3iltOVHb+QbrymtM= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 74B41F8014C; Mon, 9 May 2022 18:56:39 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 65B78F8025D; Mon, 9 May 2022 18:56:37 +0200 (CEST) Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 19D65F8014C for ; Mon, 9 May 2022 18:56:29 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 19D65F8014C Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="EtvoF8Ip" Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 62A69B8180F; Mon, 9 May 2022 16:56:28 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BFCA4C385B1; Mon, 9 May 2022 16:56:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652115387; bh=NT/NSm+ixv4NTnm+bzYUbh8NWftsy0GkupiwFftUrCw=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=EtvoF8IpfrUKSdvwW8V1qeHxhWfkW598IrjNsAqteSHaADCcEdw4x8KXDr3TbpGco +yggH/xodb4xZyPEnEeIc8mWLqDjS4iXNJXSCuBQpOQ/fSanq8ns6RptBh90cQlffa ZydWP2ygHERY2FiBF7EYP2PUX0oJed77PI+PtGtAug8JPiOmVIAJvSdJSVF0l2WgT3 wW5f+Mu+92KTr/DOaR7Nqi5mZ7fd/NZ7bLFFprVwVSRAaFOoxIjPeDgJTM5EOXSyV1 crJcVua2rcdRuwnbPejGtAvrTv7VAS/uq+3pJDH8Q5DUtKss9HWmbgKe7fxadeJAws EJ34J+Ehy52jg== Date: Mon, 9 May 2022 18:56:20 +0200 From: Mauro Carvalho Chehab To: Andi Shyti Subject: Re: [Intel-gfx] [PATCH v5 1/2] module: update dependencies at try_module_get() Message-ID: <20220509185620.05567716@coco.lan> In-Reply-To: References: X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: mauro.chehab@linux.intel.com, linux-kernel@vger.kernel.org, David Airlie , Greg KH , intel-gfx@lists.freedesktop.org, Lucas De Marchi , alsa-devel@alsa-project.org, dri-devel@lists.freedesktop.org, Takashi Iwai , Kai Vehmanen , Luis Chamberlain , Dan Williams , linux-modules@vger.kernel.org, Pierre-Louis Bossart X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" Em Thu, 5 May 2022 23:35:29 +0200 Andi Shyti escreveu: > Hi Mauro, > > [...] > > > +static int ref_module_dependency(struct module *mod, struct module *this) > > +{ > > + int ret; > > + > > + if (!this || !this->name) > > + return -EINVAL; > > + > > + if (mod == this) > > + return 0; > > + > > + mutex_lock(&module_mutex); > > + > > + ret = ref_module(this, mod); > > + > > +#ifdef CONFIG_MODULE_UNLOAD > > + if (ret) > > + goto ret; > > + > > + ret = sysfs_create_link(mod->holders_dir, > > + &this->mkobj.kobj, this->name); > > +#endif > > + > > +ret: > > + mutex_unlock(&module_mutex); > > + return ret; > > +} > > + > > /* Clear the unload stuff of the module. */ > > static void module_unload_free(struct module *mod) > > { > > @@ -841,24 +886,16 @@ void __module_get(struct module *module) > > } > > EXPORT_SYMBOL(__module_get); > > > > -bool try_module_get(struct module *module) > > +bool try_module_get_owner(struct module *module, struct module *this) > > { > > - bool ret = true; > > + int ret = __try_module_get(module); > > > > - if (module) { > > - preempt_disable(); > > - /* Note: here, we can fail to get a reference */ > > - if (likely(module_is_live(module) && > > - atomic_inc_not_zero(&module->refcnt) != 0)) > > - trace_module_get(module, _RET_IP_); > > - else > > - ret = false; > > + if (ret) > > + ref_module_dependency(module, this); > > do we care about the return value here? I don't think it should care about the return value, as a failure to create a sysfs node for the holder or to add it to the holders list is not fatal: modules can still continue working without that. Also, I opted to be conservative here: currently, not creating these doesn't cause try_module_get() to fail. I'm not sure what would be the impact if this starts to fail. So, right now, I'm opting to just ignore the return value. Perhaps in the future this could a warning (similarly to what sysfs create link does). Regards, Mauro > > Andi > > > > > - preempt_enable(); > > - } > > return ret; > > } > > -EXPORT_SYMBOL(try_module_get); > > +EXPORT_SYMBOL(try_module_get_owner); > > > > void module_put(struct module *module) > > { > > -- > > 2.35.1 Thanks, Mauro