Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 4 Jul 2023 16:35:44 +0200
From:   Jean Delvare <jdelvare@suse.de>
To:     Michal Hocko <mhocko@suse.com>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] module: print module name on refcount error
Message-ID: <20230704163544.660621f3@endymion.delvare>
In-Reply-To: <ZKQZHZt8YV0GosrZ@dhcp22.suse.cz>
References: <20230626123252.73dbc139@endymion.delvare>
        <ZJwLy5anSgFzbTUP@dhcp22.suse.cz>
        <20230704144312.032b4ddd@endymion.delvare>
        <ZKQZHZt8YV0GosrZ@dhcp22.suse.cz>
Organization: SUSE Linux
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 4 Jul 2023 15:05:33 +0200, Michal Hocko wrote:
> On Tue 04-07-23 14:43:12, Jean Delvare wrote:
> > On Wed, 28 Jun 2023 12:30:35 +0200, Michal Hocko wrote:  
> > > Would it make sense to also print the refcnt here? In our internal bug
> > > report it has turned out that this was an overflow (put missing) rather
> > > than an underflow (too many put calls). Seeing the value could give a
> > > clue about that. We had to configure panic_on_warn to capture a dump to
> > > learn more which is rather impractical.  
> > 
> > Well, other calls to module_put() or try_module_get() could happen in
> > parallel, so at the time we print refcnt, its value could be different
> > from the one which triggered the WARN.  
> 
> Racess with module_put should be impossible because all of them should
> fail, right?

Most probably yes, but after taking a deeper look at the code, I
wouldn't swear. For example delete_module() will decrement refcnt and
increment it again if the module can't actually be removed. This could
get refcnt to positive again briefly, at which point another
module_put() could succeed.

> Races with put are possible but we do not need an exact
> value to tell the difference between over and underflow, no?

Indeed not. But my other points still stand. Plus, if you really want
to know the refcnt value, it's already visible in /sys/module/*/refcnt
and lsmod.

-- 
Jean Delvare
SUSE L3 Support