Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i494840e7:Fastmail
Date:   Sat, 3 Jun 2023 12:33:48 +0300
From:   Ido Schimmel <idosch@idosch.org>
To:     NeilBrown <neilb@suse.de>
Cc:     "J. Bruce Fields" <bfields@fieldses.org>,
        Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 13/20] lockd: move lockd_start_svc() call into
 lockd_create_svc()
Message-ID: <ZHsI/H16VX9kJQX1@shredder>
References: <163816133466.32298.13831616524908720974.stgit@noble.brown>
 <163816148560.32298.15560175172815507979.stgit@noble.brown>
 <ZHoO3GRH6h/bcRjm@shredder>
 <168574130862.10905.10707785007987424080@noble.neil.brown.name>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <168574130862.10905.10707785007987424080@noble.neil.brown.name>
Precedence: bulk

On Sat, Jun 03, 2023 at 07:28:28AM +1000, NeilBrown wrote:
> From: NeilBrown <neilb@suse.de>
> Date: Sat, 3 Jun 2023 07:14:14 +1000
> Subject: [PATCH] lockd: drop inappropriate svc_get() from locked_get()
> 
> The below-mentioned patch was intended to simplify refcounting on the
> svc_serv used by locked.  The goal was to only ever have a single
> reference from the single thread.  To that end we dropped a call to
> lockd_start_svc() (except when creating thread) which would take a
> reference, and dropped the svc_put(serv) that would drop that reference.
> 
> Unfortunately we didn't also remove the svc_get() from
> lockd_create_svc() in the case where the svc_serv already existed.
> So after the patch:
>  - on the first call the svc_serv was allocated and the one reference
>    was given to the thread, so there are no extra references
>  - on subsequent calls svc_get() was called so there is now an extra
>    reference.
> This is clearly not consistent.
> 
> The inconsistency is also clear in the current code in lockd_get()
> takes *two* references, one on nlmsvc_serv and one by incrementing
> nlmsvc_users.   This clearly does not match lockd_put().
> 
> So: drop that svc_get() from lockd_get() (which used to be in
> lockd_create_svc().
> 
> Reported-by: Ido Schimmel <idosch@idosch.org>
> Fixes: b73a2972041b ("lockd: move lockd_start_svc() call into lockd_create_svc()")
> Signed-off-by: NeilBrown <neilb@suse.de>

Thanks for the quick fix. I no longer see the memory leak with this
patch.

Tested-by: Ido Schimmel <idosch@nvidia.com>