Received-SPF: pass (google.com: domain of linux-kernel+bounces-27364-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
CC: Kunwu Chan <chentao@kylinos.cn>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2] vfio: Use WARN_ON for low-probability allocation
 failure issue in vfio_pci_bus_notifier
Thread-Topic: [PATCH v2] vfio: Use WARN_ON for low-probability allocation
 failure issue in vfio_pci_bus_notifier
Thread-Index: AQHaR3z9FIdL+jn8c0+6wUQa6PWyULDa+5EggAAVQYCAAO+DsA==
Date: Tue, 16 Jan 2024 12:08:53 +0000
Message-ID: <DS0PR11MB6373DB70ED49E18C3232973FDC732@DS0PR11MB6373.namprd11.prod.outlook.com>
References: <20240115063434.20278-1-chentao@kylinos.cn>
	<DS0PR11MB6373BAF9CFEC4D67DEAAB1F7DC6C2@DS0PR11MB6373.namprd11.prod.outlook.com>
 <20240115092841.19dc32f6.alex.williamson@redhat.com>
In-Reply-To: <20240115092841.19dc32f6.alex.williamson@redhat.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?o1FbM3JfyBq3itKJ5C/DmZSkWjB9IxaICbky2N8S4TtAyg1WcYxZpdzMkJCx?=
 =?us-ascii?Q?peYfdY4k6IRAw4NRjkeAX8RzTmIJoYWT0RbBqwZHGGuzd9ge2zi5O1XBP0tW?=
 =?us-ascii?Q?GIk14KTx1olUi7eeHEjCcuXpGAZ3cp/93amCmEWVerm99ZL7UrQV/basAKg2?=
 =?us-ascii?Q?bYu8Ra/faaCJqSN04R7T/Rdpr8ZeTuq59E7DVTons1uj1EbMqVhX6oH7mu40?=
 =?us-ascii?Q?1MHXiJF2jstD2LLMfwIKoRe+9MqfOumHVIzI7D3sgz0p1E53BUWic9I0f3zM?=
 =?us-ascii?Q?lVLf2G5/D8N2rRBgpQYNNSqMxTS8EqgoRzwCWEKnLc8g5XUgwbgD6vYxcPAa?=
 =?us-ascii?Q?ydCtvBdVI3/WYYHBuWrUDjEOLDVNNY5OnK7Zff5+DdM2DMZb0mHkOnBGLdUE?=
 =?us-ascii?Q?C+LQAj6HFOLYbYSvprLXPMKf/qJHkfmTytYFBdmI1HJhJ8QS8IITwUnK1rnl?=
 =?us-ascii?Q?ZB9yLWZpVEauUT+KX8Qs4kWiJjhh3p/YE2a/DGoHa+N/VsAUmor8G4MMR0xS?=
 =?us-ascii?Q?pu+dc0Z6G+Bomn/QnH73DfI9LMDzZ55yy2sN6w94+IF7D5/A7t/56JyOjc8c?=
 =?us-ascii?Q?m63Pm6kD8uaKBGIPoT+EcK/MlMo7hEB7V09jfJekpnWHqrGXbLYyj10RP3lI?=
 =?us-ascii?Q?m6yiv91yDhZsVPN4EV2jx0b+2Hl2ygHtfGNAPCoPW8hVzMg+aKHxo+fkufd4?=
 =?us-ascii?Q?DzEgUw5AW3CFOrQy3fTFs9grl3fGfaVZmA6hYI7IfDOznicWUvHW0bBESPrM?=
 =?us-ascii?Q?y6DpXFjeyxMYSkKlHS/+BWAh9+jnoo1FEiDldT4IlFG6w/PBMKLwiZ1luZ/U?=
 =?us-ascii?Q?ZwHgbPK4UJ82thl6SeDnmmWGjntde17dapGaN242M29YpEiOzaojMUoAIFL5?=
 =?us-ascii?Q?nr+fYGB8s86oVZfsQvu5AzURDEU1b3aPt1KETe/kGKstMeajsoXoThS5b4Ch?=
 =?us-ascii?Q?+uRYxOJSm5iqe0j3Du4/9vp0DWgSLkgLxlnXSZJXCLMe2wxFpP+GUJp/1HB3?=
 =?us-ascii?Q?/NMt3apOM/Zoff5mrw4B8TlcibMgG6FNoTuBNQJvW/gEbZrX12eevnx7n+PG?=
 =?us-ascii?Q?0oFuADCqKcZXERSoACJXO2VkcKx5zUFwD9Tk2YGxVOoMJ6gQ7Msg1AxcRppM?=
 =?us-ascii?Q?o4jRwnbv2dG6X7OQfJ/7iX8IDoLvKSC/5i6cUAtrQp/CIdzw4t3Pssn6ARIq?=
 =?us-ascii?Q?6AWhu+BF/wMOWk0P9tGXYbDkQuYmmUetgS56fYoysXjZdBSMoiWSQHn2wryw?=
 =?us-ascii?Q?3fnxQywkGu0FY2Z1+eyLtERsf+P5RsxXvdmt6/cUU6o2qEzudkHiSoRweby0?=
 =?us-ascii?Q?Pb/+L0LszPwTP1/bIIBqhgXkHbRewXRlzAhyIwX0Fi/Jri10DfzS1zH0w14z?=
 =?us-ascii?Q?n4sez6gdVa4dEIyqCYi6va8irV1JLDEqH3FqWX/M9U5Ps/58jkZ2fV2ayQOC?=
 =?us-ascii?Q?+Tj9vf7t52ySh1C08VfV93JY6QqjP7luAnRIGG4/LyMzIROn1Opv9Pw+W+Gu?=
 =?us-ascii?Q?iSekv9Yo3jU4fpmHWobw4yYWQsx6OYOq2BDNSYbroHFqrqwbfjBg6r0boQCf?=
 =?us-ascii?Q?qrWo7WsdBO+ZNegZes16c34HFFQ513YJtx5Lim5f?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB6373.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b8974b54-a1d6-4752-aed3-08dc168be859
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jan 2024 12:08:53.3724
 (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: 1UHFQ7YlxpnhcCUsMkdVRJdY5E0ujtJGGUgaDbcVW5QQW4R0u/apIFauDA0unJNwyOZUdCccfLf+a4nBixN56w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5248
X-OriginatorOrg: intel.com

On Tuesday, January 16, 2024 12:29 AM, Alex Williamson wrote:
> > On Monday, January 15, 2024 2:35 PM, Kunwu Chan wrote:
> > > kasprintf() returns a pointer to dynamically allocated memory which
> > > can be NULL upon failure.
> > >
> > > This is a blocking notifier callback, so errno isn't a proper return
> > > value. Use WARN_ON to small allocation failures.
> > >
> > > Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
> > > ---
> > > v2: Use WARN_ON instead of return errno
> > > ---
> > >  drivers/vfio/pci/vfio_pci_core.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/vfio/pci/vfio_pci_core.c
> > > b/drivers/vfio/pci/vfio_pci_core.c
> > > index 1cbc990d42e0..61aa19666050 100644
> > > --- a/drivers/vfio/pci/vfio_pci_core.c
> > > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > > @@ -2047,6 +2047,7 @@ static int vfio_pci_bus_notifier(struct
> > > notifier_block *nb,
> > >  			 pci_name(pdev));
> > >  		pdev->driver_override =3D kasprintf(GFP_KERNEL, "%s",
> > >  						  vdev->vdev.ops->name);
> > > +		WARN_ON(!pdev->driver_override);
> >
> > Saw Alex's comments on v1. Curious why not return "NOTIFY_BAD" on
> > errors though less likely? Similar examples could be found in
> kvm_pm_notifier_call, kasan_mem_notifier etc.
>=20
> If the statement is that there are notifier call chains that return NOTIF=
Y_BAD, I
> would absolutely agree, but the return value needs to be examined from th=
e
> context of the caller.  BUS_NOTIFY_ADD_DEVICE is notified via bus_notify(=
) in
> device_add().  What does it accomplish to return NOTIFY_BAD in a chain th=
at
> ignores the return value?  At best we're preventing callbacks further dow=
n the
> chain from being called.
> That doesn't seem obviously beneficial either.

OK, thanks for the clarification. My curiosity came from the statement "Thi=
s is a
blocking notifier callback, so errno isn't a proper return value". Probably=
 the
commit log needs some rewording.