Thread (8 messages) 8 messages, 5 authors, 6d ago

Re: [PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

From: Arthur Kiyanovski <akiyano@amazon.com>
Date: 2026-09-21 19:06:18
Also in: lkml, stable

On Mon, 21 Sep 2026 23:42:01 +0800, Guangshuo Li [off-list ref] wrote:
ena_probe() initializes the PHC as part of ena_device_init(), but the
probe failure path does not destroy it before freeing the PHC private
data.

The normal removal path calls ena_phc_destroy() through
ena_destroy_device() before ena_phc_free(). However, if probe fails
after ena_device_init() succeeds, the error path reaches ena_phc_free()
This part:
without unregistering the PTP clock or destroying the device PHC
resources.
is inaccurate, see my explanation bellow.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index ea89619039d8..5f0864d16dd3 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -4122,6 +4122,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_device_destroy:
 	ena_com_delete_host_info(ena_dev);
 	ena_com_admin_destroy(ena_dev);
+	ena_phc_destroy(adapter);
 ena_devlink_destroy:
 	ena_devlink_free(devlink);
 err_metrics_destroy:
Thanks for resending as a series - the split and the ordering are what I
asked for.

But v2 doesn't address Sashiko's review of v1.

PHC is off during an initial probe. It only turns on if the user enables
it via devlink, and devlink is registered at the very end of a successful
ena_probe(). So on this error path phc_info->clock and
ena_dev->phc.virt_addr are both NULL and ena_phc_destroy() does nothing.

I still want the call - it keeps the probe unwind symmetric with
ena_destroy_device() - but it is a robustness change, not a fix. Please
drop Fixes: and Cc: stable, and say so in the commit message, for
example:

  No functional change: PHC is always disabled during an initial probe,
  so this only keeps the probe unwind symmetric.

Since net is for fixes, that means splitting after all - sorry for the
extra round trip:

 - the MMIO read buffer fix goes to net on its own, keeping Fixes: and
 - this patch goes to net-next, without Fixes: or Cc: stable

The MMIO fix has to land in net and propagate into net-next before you
post this one, otherwise you hit the same hunk conflict again.

Also please use --subject-prefix='PATCH net-next v3';
netdev/series_format is still warning that the target tree isn't
specified in the subject.

Thank you.

Cc: stable

Cc: stable

-- 
Arthur Kiyanovski [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help