-----Original Message----- From: liulongfang Sent: 17 October 2022 10:21 To: John Garry john.garry@huawei.com; alex.williamson@redhat.com; jgg@nvidia.com; Shameerali Kolothum Thodi shameerali.kolothum.thodi@huawei.com Cc: cohuck@redhat.com; linux-kernel@vger.kernel.org; linuxarm@openeuler.org Subject: Re: [Linuxarm] [PATCH 1/2] hisi_acc_vfio_pci: Add debugfs to migration driver
On 2022/10/14 17:20, John Garry wrote:
On 14/10/2022 03:57, Longfang Liu wrote:
+static void hisi_acc_vf_debugfs_init(struct hisi_acc_vf_core_device
*hisi_acc_vdev)
+{ + struct pci_dev *vf_pdev = hisi_acc_vdev->vf_dev; + struct device *dev = &vf_pdev->dev; + int ret;
+ if (!atomic_read(&hisi_acc_root_ref)) + hisi_acc_debugfs_root = debugfs_create_dir("hisi_vfio_acc",
NULL);
+ atomic_inc(&hisi_acc_root_ref);
This looks totally racy, such that I wonder why even bother using an atomic
for hisi_acc_root_ref.
When enabling VF, it is possible for multiple VMs to enable VF at the same time. The atomic variable is used to ensure that only one "hisi_vfio_acc" is created. When other VFs are enabled, it will not be created again, but will be used directly.
Indeed, why is hisi_acc_debugfs_root not created in the driver module init?
Because the normal function of VF is to perform encryption and decryption services, the live migration function is an auxiliary function, which no need to be used in scenarios where only encryption and decryption services are performed.
During module init, it can register ops(hisi_acc_vfio_pci_ops) that only perform encryption and decryption services, and can also register with live migration function ops(hisi_acc_vfio_pci_migrn_ops), and this debugfs only needs to register it when the the ops is hisi_acc_vfio_pci_migrn_ops.
Isn't the ops registration happens at probe()? In any case, I think you can move the hisi_acc_debugfs_root creation to module_init() as suggested above and avoid the race.
Thanks, Shameer