From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 61FAC20E3 for ; Mon, 15 Jan 2024 16:28:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="PkmLG2NR" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705336125; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xeFxX+daWDMSkrH7ZOnVTZoSM9dN+DP2BZ6PwiIA2qo=; b=PkmLG2NRJAc9a0AIjZf7fie3YJ/+A0I2SGf4oaDPQdDzFQ7kVc4BhYd7YXH7tvQuEaesAi STxUjdH2t9Dkuc7qFQEnXSf9iNUEkxtONKmR7SbcwVwc1DYQZx9WMFI9/hQq9ZfdflkDSW iPOuvBbM6hlTeqNMi9tD9pcFOkxxfgA= Received: from mail-io1-f72.google.com (mail-io1-f72.google.com [209.85.166.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-639-LTHc2U00MGCdSlaZlaYqNg-1; Mon, 15 Jan 2024 11:28:44 -0500 X-MC-Unique: LTHc2U00MGCdSlaZlaYqNg-1 Received: by mail-io1-f72.google.com with SMTP id ca18e2360f4ac-7baa6cc3af2so843339939f.2 for ; Mon, 15 Jan 2024 08:28:43 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705336123; x=1705940923; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xeFxX+daWDMSkrH7ZOnVTZoSM9dN+DP2BZ6PwiIA2qo=; b=BhlahZLQ7vFs0VjmeH7k7DvtIGCNSIffFnO2YIfd+KbqvyQBGQKHIgaQ9gV4WasA94 +H8lj6nF2pYeY3HVUNIsuMIAUonPbRWo+LO35AGoB7Qegbf3rK/eVnFBVt4F9nJKKic7 lzCfs5ow5Hzx2UmJj0Layt/Zc4elN/PNlY0XMaGQlF6xfDrZaHeE95g/32MOSbokOcxz 9WkzpP2jYZtFlcSIOuKQM3DBmQmlT+g18kOOJk8HAVjSFsB2Q/QkIZEG7uOIvvGAulrO bwC4GYoemj3btGemkQpi1e7p9mzIjKmnrdsnYIM7EVKGFkNjmeO6DLcGjcwYimIABClj eRPQ== X-Gm-Message-State: AOJu0YwCZIIHEfm6OGjLek5HKBDkyJ6Opea2MIFvGPfmtfxWHExH3ERN ziP+EHwsGQWdoZ3amTJWTAZdsO23Bnj7Q9zSlXPBlCcjyop/9bPIGpLcM4KDoG2G0h/3BRR3kfW F6JAcdbBXVNHtdlrZhyZhNYQX1CaBbbyH X-Received: by 2002:a5e:c745:0:b0:7bf:444a:1ac with SMTP id g5-20020a5ec745000000b007bf444a01acmr1850343iop.43.1705336123295; Mon, 15 Jan 2024 08:28:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IFb5zNWdn3eaZ1y+CeTf8VwTR+AfnhjZIvIzBsaZ7tYdGNeAwEtzXRKfbGWez3YXRA9BZyJEQ== X-Received: by 2002:a5e:c745:0:b0:7bf:444a:1ac with SMTP id g5-20020a5ec745000000b007bf444a01acmr1850336iop.43.1705336122981; Mon, 15 Jan 2024 08:28:42 -0800 (PST) Received: from redhat.com ([38.15.60.12]) by smtp.gmail.com with ESMTPSA id t9-20020a02c489000000b0046e7235c740sm1367316jam.106.2024.01.15.08.28.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Jan 2024 08:28:42 -0800 (PST) Date: Mon, 15 Jan 2024 09:28:41 -0700 From: Alex Williamson To: "Wang, Wei W" Cc: Kunwu Chan , "kvm@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 Message-ID: <20240115092841.19dc32f6.alex.williamson@redhat.com> In-Reply-To: References: <20240115063434.20278-1-chentao@kylinos.cn> X-Mailer: Claws Mail 4.2.0 (GTK 3.24.38; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 15 Jan 2024 15:41:02 +0000 "Wang, Wei W" 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 > > --- > > 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 = 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. If the statement is that there are notifier call chains that return NOTIFY_BAD, I would absolutely agree, but the return value needs to be examined from the 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 that ignores the return value? At best we're preventing callbacks further down the chain from being called. That doesn't seem obviously beneficial either. The scenario here is similar to that in fail_iommu_bus_notify() where they've chosen to trigger a pr_warn() if they're unable to crease sysfs entries. In fact, a pci_warn(), maybe even pci_err() might be a better alternative here than a WARN_ON(). Thanks, Alex