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;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        Alex Williamson <alex.williamson@redhat.com>,
        Greg KH <gregkh@linuxfoundation.org>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "Bradescu, Roxana" <roxabee@google.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 2/2] kvm/vfio: avoid bouncing the mutex when adding and
 deleting groups
Thread-Topic: [PATCH v3 2/2] kvm/vfio: avoid bouncing the mutex when adding
 and deleting groups
Thread-Index: AQHZtqT6WmtjXEfJ/0GeKcImZTi8I6/AlmDwgAAL0ICAAVYmUA==
Date:   Thu, 20 Jul 2023 02:36:16 +0000
Message-ID: <BN9PR11MB52763F0990CD23DB63AC36578C3EA@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20230714224538.404793-1-dmitry.torokhov@gmail.com>
 <20230714224538.404793-2-dmitry.torokhov@gmail.com>
 <BN9PR11MB5276BCE644231C440053AA118C39A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <ZLd+X7Zcnlq52Tp+@google.com>
In-Reply-To: <ZLd+X7Zcnlq52Tp+@google.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?+yzhI5NskqEL0X/n/S9C+rwjgDNLN5APOUefTVTw2HJUaVCxLJotIdyjMhDZ?=
 =?us-ascii?Q?sxMWoQ5ceNlNGRNXmo8MjHb9zvcCzh7axmWkQvFBp5xm0QszdNqRQL2BOxSw?=
 =?us-ascii?Q?kV13mDl4PGdsDhGRrmR4/5PBZNmVCxtTzTy9JN3jIhFdF0hmKvqdkq+ro7ng?=
 =?us-ascii?Q?QJwo5EgvU7LG06YNPhb6Z0wp8pRt993+e+XSjtjXuvLve6OmdxiJ35azENuI?=
 =?us-ascii?Q?ZdwFc7JunyD1AY3xjulvR13XJRtQc2QMbQaf72BzKvgHIuDUaelolZ5JnyPH?=
 =?us-ascii?Q?MRuvikMZ1Z8MZiCvy7XXJvOtDOyqaMXLmeh4GnreY/St2nz624vGzVj9KOWF?=
 =?us-ascii?Q?9q4kpPVPlFxOZhzisuefzow/O2LShCWzElcHJ7iRcxnoZ9Yd+/VLVs5dITto?=
 =?us-ascii?Q?JXBtU3Ca2uQjrw/duug1iPLHlPW3clPWKZsxBygMYwr/lcMTdlu9CLf76Fu7?=
 =?us-ascii?Q?v3vEAFLvWN5dwL+avwO3NGmHaL3NSE6Rx9srqYEY202WVmw7BsXqFbuajgeI?=
 =?us-ascii?Q?7RocsJufcRi3aKXoBQgW6s304Quap1VPlgQ9miYc4vngy9LHeUeKpqR7FdC0?=
 =?us-ascii?Q?57cVYZpyxWQhQYNmAthvIib9H52iJbnHe/nzciSPN+l9nCLF8aA+3uvciDsu?=
 =?us-ascii?Q?v7G3WW98xeZ8+ui9IF+uE/ijX2AZEKopdiBDW7FXuDTd/PVGE0hSLyjGSay1?=
 =?us-ascii?Q?wYCOkqeozkMIN0LNKNLeD4a+SmBgwz57EVs9vIA2qEFp1WB9o0O8d1zPDjSu?=
 =?us-ascii?Q?t1ds6jMRl8NOBc4cu0yTExDgnRaCgqNm9+cXd7mW5seDeP0wai51EZtwBHqu?=
 =?us-ascii?Q?scsIw7Fh0l73C+fbWMZli/mbQKmpI8q6lq8AVDofuaMzyGxfffONQd3VcpLs?=
 =?us-ascii?Q?p04X0LjXPEachN25sACgs0ZqlUWEpGnvLIwk/Yono+ncXQiWsiHxlwU7frxS?=
 =?us-ascii?Q?SIw/Sg/gGEWZ22nAvjM+7Y3rdaoMAuLXAqc7wUB616kxHeQlw37rzVq9yb7e?=
 =?us-ascii?Q?ZyEHsQ5WnVl2iAruXscIgn8btlf0RT+uUjkEDNEAGseRmb64d0kwHUYIerKq?=
 =?us-ascii?Q?Bl20KpLut/LlDhsPG0/IHni5AvIJuYBvUrHdgedPD/7EClMtJDPpdCYF9LB9?=
 =?us-ascii?Q?7VzWk7kkeHu5ITfcYfcoTdfVdRyY0pBfv6py1h/paSyn+QXPEJuHcsc8x/xg?=
 =?us-ascii?Q?Um4LUSZxi7iviQtaDfBl1FNw2lw6ehsZnfaxohLdzIc8m91AFEhovKYcK7Ko?=
 =?us-ascii?Q?OgQTiegeAfz6Mw1ViAMDGhWTsmyg7iyunUw2rRKJxt3ZLbBnIz/CuCGhk0q+?=
 =?us-ascii?Q?yuShgFuG9TzGDkG6UVmZe/VanmeC56WViTXNW5CJHgkSFMj46vr5+2UDC/+1?=
 =?us-ascii?Q?Xv/6RZNFBtmDQz/XCS0hSQ1H7XZkE4Lds14UKBHEUvhG+LKNPpreh+5FCAGg?=
 =?us-ascii?Q?y1kYlaFKRa74Lo3ESaooe7FWg6cyTnTlm9TyjNmbFYJ1zbzWoQ9UZD4VGCbp?=
 =?us-ascii?Q?5TInFXMrCUuKSHJk5OqC6Yszser8/wZJHNEiL7trRdxRFHbsjtlo/rdoKBof?=
 =?us-ascii?Q?S+FGa9RyIRtG8QW+JOADyoWwkia5EaZ9UaSgV5C8?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ee566edc-de9a-473d-aeb6-08db88ca17da
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jul 2023 02:36:16.7983
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: b4h5EQNrKK8tBQC1NPQj3SmHg3e849Dj+ro0nnbb03yoD2m00o4J2alCL+X337USJSM6hMs4jNZYr5CUXmKQ9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6712
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Sent: Wednesday, July 19, 2023 2:11 PM
>=20
> On Wed, Jul 19, 2023 at 05:32:27AM +0000, Tian, Kevin wrote:
> > > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Sent: Saturday, July 15, 2023 6:46 AM
> > >
> > > @@ -165,30 +161,26 @@ static int kvm_vfio_group_add(struct
> kvm_device
> > > *dev, unsigned int fd)
> > >  	list_for_each_entry(kvg, &kv->group_list, node) {
> > >  		if (kvg->file =3D=3D filp) {
> > >  			ret =3D -EEXIST;
> > > -			goto err_unlock;
> > > +			goto out_unlock;
> > >  		}
> > >  	}
> > >
> > >  	kvg =3D kzalloc(sizeof(*kvg), GFP_KERNEL_ACCOUNT);
> > >  	if (!kvg) {
> > >  		ret =3D -ENOMEM;
> > > -		goto err_unlock;
> > > +		goto out_unlock;
> > >  	}
> > >
> > > -	kvg->file =3D filp;
> > > +	kvg->file =3D get_file(filp);
> >
> > Why is another reference required here?
>=20
> Because the function now has a single exit point and the original
> reference is dropped unconditionally on exit. It looks cleaner than
> checking for non-zero "ret" and deciding whether the reference should be
> dropped or kept.
>=20

A comment is appreciated. otherwise,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>