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;
Subject: Re: [Linuxarm] [PATCH 1/2] hisi_acc_vfio_pci: Add debugfs to
 migration driver
To:     John Garry <john.garry@huawei.com>, <alex.williamson@redhat.com>,
        <jgg@nvidia.com>, <shameerali.kolothum.thodi@huawei.com>
CC:     <cohuck@redhat.com>, <linux-kernel@vger.kernel.org>,
        <linuxarm@openeuler.org>
References: <20221014025757.39415-1-liulongfang@huawei.com>
 <20221014025757.39415-2-liulongfang@huawei.com>
 <42d12308-4777-47a7-a1ae-50dfada050cf@huawei.com>
From:   liulongfang <liulongfang@huawei.com>
Message-ID: <7c041671-71bf-d558-d47a-70296fe5c30e@huawei.com>
Date:   Mon, 17 Oct 2022 17:20:34 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <42d12308-4777-47a7-a1ae-50dfada050cf@huawei.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.

> Thanks,
> John
> .
>