[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN9PR11MB5276A78546BD2B38068385E18CBA9@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Thu, 30 Jun 2022 08:28:07 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Lu Baolu <baolu.lu@...ux.intel.com>,
Joerg Roedel <joro@...tes.org>, Steve Wahl <steve.wahl@....com>
CC: David Woodhouse <dwmw2@...radead.org>,
Jerry Snitselaar <jsnitsel@...hat.com>,
Mike Travis <mike.travis@....com>,
Dimitri Sivanich <sivanich@....com>,
"Anderson, Russ" <russ.anderson@....com>,
"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v1 3/6] iommu/vt-d: Refactor iommu information of each
domain
> From: Lu Baolu <baolu.lu@...ux.intel.com>
> Sent: Saturday, June 25, 2022 8:52 PM
>
> +struct iommu_domain_info {
> + struct intel_iommu *iommu;
> + unsigned int refcnt;
> + u16 did;
> +};
> +
> struct dmar_domain {
> int nid; /* node id */
> -
> - unsigned int iommu_refcnt[DMAR_UNITS_SUPPORTED];
> - /* Refcount of devices per iommu */
> -
> -
> - u16 iommu_did[DMAR_UNITS_SUPPORTED];
> - /* Domain ids per IOMMU. Use u16
> since
> - * domain ids are 16 bit wide
> according
> - * to VT-d spec, section 9.3 */
It'd make sense to keep the comments when moving above fields.
> + spin_lock(&iommu->lock);
> + curr = xa_load(&domain->iommu_array, iommu->seq_id);
> + if (curr) {
> + curr->refcnt++;
> + kfree(info);
> + goto success;
Not a fan of adding a label for success. Just putting two lines (unlock+
return) here is clearer.
> + ret = xa_err(xa_store(&domain->iommu_array, iommu->seq_id,
> + info, GFP_ATOMIC));
check xa_err in separate line.
>
> static void domain_detach_iommu(struct dmar_domain *domain,
> struct intel_iommu *iommu)
> {
> - int num;
> + struct iommu_domain_info *info;
>
> spin_lock(&iommu->lock);
> - domain->iommu_refcnt[iommu->seq_id] -= 1;
> - if (domain->iommu_refcnt[iommu->seq_id] == 0) {
> - num = domain->iommu_did[iommu->seq_id];
> - clear_bit(num, iommu->domain_ids);
> + info = xa_load(&domain->iommu_array, iommu->seq_id);
> + if (--info->refcnt == 0) {
> + clear_bit(info->did, iommu->domain_ids);
> + xa_erase(&domain->iommu_array, iommu->seq_id);
> domain_update_iommu_cap(domain);
> - domain->iommu_did[iommu->seq_id] = 0;
> + kfree(info);
domain->nid may still point to the node of the removed iommu.
Powered by blists - more mailing lists