Message ID | 20231025110140.2034650-1-dario.binacchi@amarulasolutions.com |
---|---|
State | Rejected |
Headers | show |
Series | iommu/tegra-smmu: fix error checking for debugfs_create_dir() | expand |
On 2023/10/25 19:01, Dario Binacchi wrote: > The return value of debugfs_create_dir() should be checked using the > IS_ERR() function. > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > --- > > drivers/iommu/tegra-smmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > index e445f80d0226..cd1d80c4c673 100644 > --- a/drivers/iommu/tegra-smmu.c > +++ b/drivers/iommu/tegra-smmu.c > @@ -1056,7 +1056,7 @@ DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients); > static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) > { > smmu->debugfs = debugfs_create_dir("smmu", NULL); > - if (!smmu->debugfs) > + if (IS_ERR(smmu->debugfs)) > return; This check can be removed, as debugfs_create_file() can handle the case where @parent is an error pointer. > > debugfs_create_file("swgroups", S_IRUGO, smmu->debugfs, smmu, Best regards, baolu
On Wed, Oct 25, 2023 at 07:25:50PM +0800, Baolu Lu wrote: > On 2023/10/25 19:01, Dario Binacchi wrote: > > The return value of debugfs_create_dir() should be checked using the > > IS_ERR() function. > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > --- > > > > drivers/iommu/tegra-smmu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > > index e445f80d0226..cd1d80c4c673 100644 > > --- a/drivers/iommu/tegra-smmu.c > > +++ b/drivers/iommu/tegra-smmu.c > > @@ -1056,7 +1056,7 @@ DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients); > > static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) > > { > > smmu->debugfs = debugfs_create_dir("smmu", NULL); > > - if (!smmu->debugfs) > > + if (IS_ERR(smmu->debugfs)) > > return; > > This check can be removed, as debugfs_create_file() can handle the case > where @parent is an error pointer. A patch for this has been in linux-next for a few weeks, see: commit f7da9c081517daba70f9f9342e09d7a6322ba323 Author: Jinjie Ruan <ruanjinjie@huawei.com> Date: Fri Sep 1 15:30:56 2023 +0800 iommu/tegra-smmu: Drop unnecessary error check for for debugfs_create_dir() The debugfs_create_dir() function returns error pointers. It never returns NULL. As Baolu suggested, this patch removes the error checking for debugfs_create_dir in tegra-smmu.c. This is because the DebugFS kernel API is developed in a way that the caller can safely ignore the errors that occur during the creation of DebugFS nodes. The debugfs APIs have a IS_ERR() judge in start_creating() which can handle it gracefully. So these checks are unnecessary. Fixes: d1313e7896e9 ("iommu/tegra-smmu: Add debugfs support") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Suggested-by: Baolu Lu <baolu.lu@linux.intel.com> Acked-by: Thierry Reding <treding@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Link: https://lore.kernel.org/r/20230901073056.1364755-1-ruanjinjie@huawei.com Signed-off-by: Joerg Roedel <jroedel@suse.de> Thierry
Hello Thierry, On Mon, Oct 30, 2023 at 4:30 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Oct 25, 2023 at 07:25:50PM +0800, Baolu Lu wrote: > > On 2023/10/25 19:01, Dario Binacchi wrote: > > > The return value of debugfs_create_dir() should be checked using the > > > IS_ERR() function. > > > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> > > > --- > > > > > > drivers/iommu/tegra-smmu.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > > > index e445f80d0226..cd1d80c4c673 100644 > > > --- a/drivers/iommu/tegra-smmu.c > > > +++ b/drivers/iommu/tegra-smmu.c > > > @@ -1056,7 +1056,7 @@ DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients); > > > static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) > > > { > > > smmu->debugfs = debugfs_create_dir("smmu", NULL); > > > - if (!smmu->debugfs) > > > + if (IS_ERR(smmu->debugfs)) > > > return; > > > > This check can be removed, as debugfs_create_file() can handle the case > > where @parent is an error pointer. > > A patch for this has been in linux-next for a few weeks, see: > > commit f7da9c081517daba70f9f9342e09d7a6322ba323 > Author: Jinjie Ruan <ruanjinjie@huawei.com> > Date: Fri Sep 1 15:30:56 2023 +0800 > > iommu/tegra-smmu: Drop unnecessary error check for for debugfs_create_dir() > > The debugfs_create_dir() function returns error pointers. > It never returns NULL. > > As Baolu suggested, this patch removes the error checking for > debugfs_create_dir in tegra-smmu.c. This is because the DebugFS kernel API > is developed in a way that the caller can safely ignore the errors that > occur during the creation of DebugFS nodes. The debugfs APIs have > a IS_ERR() judge in start_creating() which can handle it gracefully. So > these checks are unnecessary. > > Fixes: d1313e7896e9 ("iommu/tegra-smmu: Add debugfs support") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > Suggested-by: Baolu Lu <baolu.lu@linux.intel.com> > Acked-by: Thierry Reding <treding@nvidia.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Link: https://lore.kernel.org/r/20230901073056.1364755-1-ruanjinjie@huawei.com > Signed-off-by: Joerg Roedel <jroedel@suse.de> > Thanks for the explanation, regards Dario > Thierry
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index e445f80d0226..cd1d80c4c673 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -1056,7 +1056,7 @@ DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients); static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) { smmu->debugfs = debugfs_create_dir("smmu", NULL); - if (!smmu->debugfs) + if (IS_ERR(smmu->debugfs)) return; debugfs_create_file("swgroups", S_IRUGO, smmu->debugfs, smmu,
The return value of debugfs_create_dir() should be checked using the IS_ERR() function. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- drivers/iommu/tegra-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)