Received: by 2002:a6b:fb09:0:0:0:0:0 with SMTP id h9csp826814iog; Mon, 13 Jun 2022 13:56:57 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwBH8929JorXNFmRpGqleh2JbaKWDdNfNeZcaAyLicQHol8Fao4IE+yZ7OuN24ODUZhSkS2 X-Received: by 2002:a17:906:7049:b0:70c:a5fe:d4f8 with SMTP id r9-20020a170906704900b0070ca5fed4f8mr1347391ejj.493.1655153817404; Mon, 13 Jun 2022 13:56:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1655153817; cv=none; d=google.com; s=arc-20160816; b=XDZBTOYRzmZ5lBX+BX1STFCGpP6dqKYHMtdIrSBQ4wAoawcEnH1Ccbmz92ogeJ02Fb QJwZdqicbBiIk9ZDUWK9CwyQjOOJwxkTPzzxVHkhudEStiaxXACgSGoUSjWVrbWiadkK 8ink9Daf57y5rIdJH3av6iHlS7yXtxnNQPgVuPwKIeNWSqoWmdaY5/g+ko78ULk6sA5t +rYFVfG+YupfxsFqAmh2cHMRfD6eve+RPIo1S6N+nyE4LtnuDrQhBJNWhKbNHNn0Mn3G ESQOyNj4u2Eqte2eQu3SidLmfSv9tDE8vPVFXlwlo4JePMv9akyJySxRtBFcgqdg/49T r5hg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature :dkim-filter; bh=HWaF1QNmxsIR85mJlUFoIZu1n8vlMMKl1k718oV97FY=; b=QiZ8hk5hZ5qs4v3wIwtjtY4Otb8MXMvqlgYIfjs3KIjEQhbOhNosBfLmzFh1neEAqc x1Ou5sU90mXM9LkCDZlgMrGZWh6fnihMj87nbT1ko4lS9w20jolPr7iinPv8bQTEu9KJ +fIgb1NuHfDGS4C4ZxeEK8ZMIIlH17MqLuQah0F+bYLD7VHDyfoMJ/1Zp5mBMZJpdQkU yP/Hr/vp1zcJUtG16ccT4mSgutau1wFDYC/Fm28KWq0GFcKcE1i3BHyAuA88kUbG1qjt Z2vCvvCFNHgy9iCHDOqUNCsE9lT/PSA0gc9lyf/7hDRqppqDPykKmrOZWwVkRXPsBIJV TFvg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=Rgkl+iIp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m13-20020a056402430d00b00427ba3579f5si10650043edc.26.2022.06.13.13.56.32; Mon, 13 Jun 2022 13:56:57 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=Rgkl+iIp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.microsoft.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241622AbiFMUy3 (ORCPT + 99 others); Mon, 13 Jun 2022 16:54:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348470AbiFMUxA (ORCPT ); Mon, 13 Jun 2022 16:53:00 -0400 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 65BA7D59 for ; Mon, 13 Jun 2022 13:12:44 -0700 (PDT) Received: from sequoia (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id AF05120C29A6; Mon, 13 Jun 2022 13:12:43 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AF05120C29A6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1655151164; bh=HWaF1QNmxsIR85mJlUFoIZu1n8vlMMKl1k718oV97FY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Rgkl+iIpE6OKLauIfR74wAB6KRvdMtoESL9vSIufW93thHl16YFEvhVZifRVbiUu0 okM9FabZGQj+SsIFQ/0x3Z5PzzvEL2NBkyRKzsuqZAM9kUrt+C5Exm+Jm8AiQyQKhp 0eiBsjty9nTvwQ25AoDx290CPSd7qIp/f1Cp8d50= Date: Mon, 13 Jun 2022 15:12:41 -0500 From: Tyler Hicks To: Dominique Martinet Cc: Christian Schoenebeck , Eric Van Hensbergen , Latchesar Ionkov , v9fs-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH 06/06] 9p fid refcount: cleanup p9_fid_put calls Message-ID: <20220613201241.GH7401@sequoia> References: <20220612085330.1451496-1-asmadeus@codewreck.org> <20220612085330.1451496-7-asmadeus@codewreck.org> <20220613175509.GE7401@sequoia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-19.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022-06-14 04:50:45, Dominique Martinet wrote: > Thanks for the reviews, > > Tyler Hicks wrote on Mon, Jun 13, 2022 at 12:55:09PM -0500: > > > @@ -189,13 +197,13 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, > > > else > > > uname = v9ses->uname; > > > > > > - root_fid = p9_client_attach(v9ses->clnt, NULL, uname, uid, > > > - v9ses->aname); > > > - if (IS_ERR(root_fid)) > > > - return root_fid; > > > + fid = p9_client_attach(v9ses->clnt, NULL, uname, uid, > > > > To keep the readability benefits in my "9p: Track the root fid with its > > own variable during lookups" patch, I think root_fid should be assigned > > here and then used in the error check and return statement. > > > > > + v9ses->aname); > > > + if (IS_ERR(fid)) > > > + return fid; > > > > > > - p9_fid_get(root_fid); > > > - v9fs_fid_add(dentry->d_sb->s_root, root_fid); > > > + root_fid = p9_fid_get(fid); > > > + v9fs_fid_add(dentry->d_sb->s_root, &fid); > > > > root_fid should be used in the two lines above, too. > > This actually was the only place where we still want to use the root_fid > after calling v9fs_fid_add; if we keep root_fid above we need to do > something like > > fid = p9_fid_get(root_fid); > v9fs_Fid_add(dentry->d_sb->s_root, &root_fid); > root_fid = fid; > // fid = NULL; ? not strictly needed as we set it again shortly afterwards > > which I wanted to avoid, but I guess I don't mind strongly either way -- > pick your poison. Ah, very good point. I missed that subtly. > I could also just keep v9fs_fid_add as a non-stealing version, but I > think it's better that way as it strongly signal that we stashed that > ref away and shouldn't use the fid anymore unless another ref was > obtained through fid_get like we do here. > (I was actually tempted to do the same with p9_fid_put, but checking > other kernel "put"s I didn't see any code that does this so I refrained > from that churn) I like the stealing version. Lets go with it and keep this patch as-is. Please feel free to slap this tag on it: Reviewed-by: Tyler Hicks Tyler > > -- > Dominique >