Messages in this thread |  | | From | "Yang, Shunyong" <> | Subject | Re: [PATCH v3 2/2] iommu/amd: Add basic debugfs infrastructure for AMD IOMMU | Date | Wed, 18 Apr 2018 05:58:48 +0000 |
| |
Hi, Gary and Sohil,
On Tue, 2018-04-17 at 13:38 -0400, Hook, Gary wrote: > On 4/13/2018 8:08 PM, Mehta, Sohil wrote: > > > > On Fri, 2018-04-06 at 08:17 -0500, Gary R Hook wrote: > > > > > > > > > + > > > +void amd_iommu_debugfs_setup(struct amd_iommu *iommu) > > > +{ > > > + char name[MAX_NAME_LEN + 1]; > > > + struct dentry *d_top; > > > + > > > + if (!debugfs_initialized()) > > Probably not needed. > Right.
When will this check is needed? IMO, this function is to check debugfs ready status before we want to use debugfs. I just want to understand when we should use debugfs_initialized();
Thanks. Shunyong.
> > > > > > > > > > > + return; > > > + > > > + mutex_lock(&amd_iommu_debugfs_lock); > > > + if (!amd_iommu_debugfs) { > > > + d_top = iommu_debugfs_setup(); > > > + if (d_top) > > > + amd_iommu_debugfs = > > > debugfs_create_dir("amd", d_top); > > > + } > > > + mutex_unlock(&amd_iommu_debugfs_lock); > > > > You can do the above only once if you iterate over the IOMMUs here > > instead of doing it in amd_iommu_init. > I'm not sure it matters, given the finite number of IOMMUs in a > system, > and the fact that this work is done exactly once. However, removal of > a > lock is fine thing, so I'll move this around. > > > > > > > > > > > + if (amd_iommu_debugfs) { > > > + snprintf(name, MAX_NAME_LEN, "iommu%02d", iommu- > > > > > > > > index); > > > + iommu->debugfs = debugfs_create_dir(name, > > > + amd_iommu_de > > > bugf > > > s); > > > + if (!iommu->debugfs) { > > > + debugfs_remove_recursive(amd_iommu_debug > > > fs); > > > + amd_iommu_debugfs = NULL; > > > + } > > > + } > > > +} > > -Sohil > > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu
|  |