Re: [PATCH] iommu/qcom: add missing put_device() call in qcom_iommu_of_xlate()
… > +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c … > @@ -595,6 +597,7 @@ static int qcom_iommu_of_xlate(struct device *dev, struct > of_phandle_args *args) >* banks are ok, but multiple devices are not: >*/ > if (WARN_ON(qcom_iommu != dev_iommu_priv_get(dev))) > + put_device(_pdev->dev); > return -EINVAL; > } * Would there be a need to use curly brackets for such an if branch? * I suggest to add a jump target so that a bit of common exception handling code can be better reused for this function implementation. Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] vfio: type1: fix kthread use case
… > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -2798,7 +2798,7 @@ static int vfio_iommu_type1_dma_rw_chunk … > - bool kthread = current->mm == NULL; > + bool kthread_load_mm; > size_t offset; How do you think about to reduce the scope for such variables? https://refactoring.com/catalog/reduceScopeOfVariable.html … > @@ -2812,11 +2812,12 @@ static int vfio_iommu_type1_dma_rw_chunk … > if (!mm) > return -EPERM; … > + kthread_load_mm = current->flags & PF_KTHREAD && > + current->mm == NULL; … Would you like to apply a more succinct code variant? + kthread_load_mm = current->flags & PF_KTHREAD && !current->mm; Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] vfio: type1: fix kthread use case
… > +++ b/drivers/vfio/vfio_iommu_type1.c … > @@ -2812,11 +2815,10 @@ static int vfio_iommu_type1_dma_rw_chunk … > if (!mm) > return -EPERM; > > - if (kthread) > + if (kthread && use_mm) Can another design approach make sense here? + bool thread_use_mm = ((current->flags & PF_KTHREAD) && !current->mm); + if (thread_use_mm) > kthread_use_mm(mm); … Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] vfio: type1: fix kthread use case
>> Can it be helpful to convert initialisations for these variables >> into later assignments? > > Perhaps. Then it looks like the below. … > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -2798,9 +2798,12 @@ static int vfio_iommu_type1_dma_rw_chunk … > - bool kthread = current->mm == NULL; > + bool kthread; > + bool use_mm; I would prefer the following variable declarations then. + bool kthread, use_mm; > size_t offset; > > + kthread = current->flags & PF_KTHREAD; > + use_mm = current->mm == NULL; I propose to move such assignments directly before the corresponding check. … > if (!mm) > return -EPERM; + kthread = current->flags & PF_KTHREAD; + use_mm = !current->mm; > - if (kthread) > + if (kthread && use_mm) > kthread_use_mm(mm); … Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] vfio: type1: fix kthread use case
… > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -2798,7 +2798,8 @@ static int vfio_iommu_type1_dma_rw_chunk … > - bool kthread = current->mm == NULL; > + bool kthread = current->flags & PF_KTHREAD; > + bool use_mm = current->mm == NULL; … Can it be helpful to convert initialisations for these variables into later assignments? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu: Improve exception handling in iommu_group_alloc()
> Improve the exception handling to free the resources correctly when > failed to allocate an iommu group. I propose to avoid the specification of duplicate function calls. Will it become helpful to add a few jump targets? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=f359287765c04711ff54fbd11645271d8e5ff763#n455 > --- > drivers/iommu/iommu.c | 10 -- I suggest to replace the triple dashes before this diffstat by a blank line. Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: iommu: Improve exception handling in iommu_group_alloc()
>> Do you disagree to the application of the Linux coding style then >> for the recommended exception handling? > > No, that's not what I mean. My point is the exception handling in this > patch is simple and no need to add 'goto' statement which does not help > to improve readability. Do we come along different programming imaginations? > And I agree it is helpful for the cases where a function exits from multiple > locations > and more same cleanup work need to do. Can this view fit also to calls of the function “kobject_put”? https://lore.kernel.org/patchwork/patch/1251326/ https://lore.kernel.org/linux-iommu/8bfd018ef4add083a35a6a94a8da045cf3af51b6.1591063271.git.baolin.w...@linux.alibaba.com/ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/iommu.c?id=7cc31613734c4870ae32f5265d576ef296621343#n478 https://elixir.bootlin.com/linux/v5.7/source/drivers/iommu/iommu.c#L478 Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: iommu: Improve exception handling in iommu_group_alloc()
>> * I suggest to avoid the specification of duplicate function calls. >> Will it be helpful to add a few jump targets? >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162#n455 > > I don't think it is helpful or readable to add some jump targets here, > since the exception handling is very simple here. Do you disagree to the application of the Linux coding style then for the recommended exception handling? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Improve exception handling in iommu_group_alloc()
> Optimize the error handling to free the resources correctly when > failed to allocate an iommu group. * I would not categorise the desired completion of exception handling as a software optimisation. * Would you like to add the tag “Fixes” to the commit message? * I suggest to avoid the specification of duplicate function calls. Will it be helpful to add a few jump targets? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162#n455 Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/qcom: Fix local_base status check
> … >> +++ b/drivers/iommu/qcom_iommu.c >> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct >> platform_device *pdev) >> qcom_iommu->dev = dev; >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); … >> qcom_iommu->local_base = devm_ioremap_resource(dev, res); … > Please take another look at a corresponding usage example. I would like to point another possibility out for desirable software evolution. How do you think about to call a “known” wrapper function instead? devm_platform_get_and_ioremap_resource https://elixir.bootlin.com/linux/v5.7-rc1/source/drivers/base/platform.c#L66 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/base/platform.c Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/qcom: Fix local_base status check
> The function qcom_iommu_device_probe() does not perform sufficient
> error checking after executing devm_ioremap_resource(), which can
> result in crashes if a critical error path is encountered.
Your update suggestion will be rechecked once more.
* Can it be that the patch would need a higher version number
according to previous review comments?
* Would you like to adjust the patch subject?
…
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct
> platform_device *pdev)
> qcom_iommu->dev = dev;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (res)
I find the deletion of this check appropriate.
> + if (res) {
> qcom_iommu->local_base = devm_ioremap_resource(dev, res);
But I do not see a need to preserve such a check because this function
performs input parameter validation.
https://elixir.bootlin.com/linux/v5.7-rc1/source/lib/devres.c#L116
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/devres.c?id=50cc09c18985eacbbd666acfd7be2391394733f5#n116
Please take another look at a corresponding usage example.
Regards,
Markus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/qcom: Fix local_base status check
> Release resources when exiting on error.
I have got doubts that such a change description fits to
the proposed source code adjustment.
…
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -813,8 +813,11 @@ static int qcom_iommu_device_probe(struct
> platform_device *pdev)
> qcom_iommu->dev = dev;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (res)
> + if (res) {
> qcom_iommu->local_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(qcom_iommu->local_base))
> + return PTR_ERR(qcom_iommu->local_base);
> + }
>
> qcom_iommu->iface_clk = devm_clk_get(dev, "iface");
…
Will the commit message be improved any further?
Would you like to add the tag “Fixes”?
Regards,
Markus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
iommu/rockchip: Checking a device_link_add() call in rk_iommu_add_device()
Hello, I tried another script for the semantic patch language out. This source code analysis approach points out that the implementation of the function “rk_iommu_add_device” contains still an unchecked call of the function “device_link_add”. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/iommu/rockchip-iommu.c?id=3ef845da3c3b180ddd386e228ac3228d84a522d3#n1075 https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/iommu/rockchip-iommu.c#L1071 How do you think about to improve it? Which error code would you like to return for a failed device link addition at this place? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
iommu/exynos: Checking a device_link_add() call in exynos_iommu_add_device()
Hello, I tried another script for the semantic patch language out. This source code analysis approach points out that the implementation of the function “exynos_iommu_add_device” contains still an unchecked call of the function “device_link_add”. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/exynos-iommu.c?id=0e2adab6cf285c41e825b6c74a3aa61324d1132c#n1253 https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/iommu/exynos-iommu.c#L1253 How do you think about to improve it? * Which error code would you like to return for a failed device link addition at this place? * Will it be needed to delete any links as exception handling? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: fix leaked of_node references
> The call to of_parse_phandle returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
Can a splitting of this information into two sentences help?
> 581 static int mtk_iommu_probe(struct platform_device *pdev)
> 582 {
> ...
I suggest to reconsider such a commit description once more.
Would it be better to mention that another function call should be added
in two if branches so that the exception handling would be completed
(instead of copying source code where a software update should become
clear also from the provided diff hunk)?
Regards,
Markus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: fix leaked of_node references
> The call to of_parse_phandle returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
Can a splitting of this information into two sentences help?
> 581 static int mtk_iommu_probe(struct platform_device *pdev)
> 582 {
> ...
I suggest to reconsider such a commit description once more.
Would it be better to mention that another function call should be added
in two if branches so that the exception handling would be completed
(instead of copying source code where a software update should become
clear also from the provided diff hunk)?
Regards,
Markus
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: ARM-SMMU: Delete error messages for a failed memory allocation in three functions
>> Date: Sat, 20 Jan 2018 15:30:17 +0100 >> >> Omit extra messages for a memory allocation failure in these functions. > > Why? Do you find the wording “WARNING: Possible unnecessary 'out of memory' message” (from the script “checkpatch.pl”) more reasonable? > This may as well be "delete some stuff because I feel like it". Do you find the Linux allocation failure report insufficient in this use case? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] Intel-IOMMU: Delete an error message for a failed memory allocation in init_dmars()
>> Date: Sat, 20 Jan 2018 13:51:49 +0100 >> >> Omit an extra message for a memory allocation failure in this function. >> >> This issue was detected by using the Coccinelle software. >> >> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> > > NACK on this one and the other two IOMMU patches you sent. Do you need any more background information for this general transformation pattern? > The messages give the user/developer useful information about what the memory > that failed to allocate would have been used for. Do you find the Linux allocation failure report insufficient for this use case? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] ARM-SMMU: Delete error messages for a failed memory allocation in three functions
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 20 Jan 2018 15:30:17 +0100
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/arm-smmu-v3.c | 9 +++--
drivers/iommu/arm-smmu.c| 9 +++--
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f122071688fd..5c2a7103d494 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2134,10 +2134,8 @@ static int arm_smmu_init_l1_strtab(struct
arm_smmu_device *smmu)
void *strtab = smmu->strtab_cfg.strtab;
cfg->l1_desc = devm_kzalloc(smmu->dev, size, GFP_KERNEL);
- if (!cfg->l1_desc) {
- dev_err(smmu->dev, "failed to allocate l1 stream table desc\n");
+ if (!cfg->l1_desc)
return -ENOMEM;
- }
for (i = 0; i < cfg->num_l1_ents; ++i) {
arm_smmu_write_strtab_l1_desc(strtab, >l1_desc[i]);
@@ -2828,10 +2826,9 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
bool bypass;
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
- if (!smmu) {
- dev_err(dev, "failed to allocate arm_smmu_device\n");
+ if (!smmu)
return -ENOMEM;
- }
+
smmu->dev = dev;
if (dev->of_node) {
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 78d4c6b8f1ba..a4da4a870a2e 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2048,10 +2048,9 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
int num_irqs, i, err;
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
- if (!smmu) {
- dev_err(dev, "failed to allocate arm_smmu_device\n");
+ if (!smmu)
return -ENOMEM;
- }
+
smmu->dev = dev;
if (dev->of_node)
@@ -2084,10 +2083,8 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
smmu->irqs = devm_kzalloc(dev, sizeof(*smmu->irqs) * num_irqs,
GFP_KERNEL);
- if (!smmu->irqs) {
- dev_err(dev, "failed to allocate %d irqs\n", num_irqs);
+ if (!smmu->irqs)
return -ENOMEM;
- }
for (i = 0; i < num_irqs; ++i) {
int irq = platform_get_irq(pdev, i);
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/dmar: Delete an error message for a failed memory allocation in dmar_alloc_pci_notify_info()
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 20 Jan 2018 14:55:21 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/dmar.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 9a7ffd13c7f0..da6b121590cc 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -150,8 +150,6 @@ dmar_alloc_pci_notify_info(struct pci_dev *dev, unsigned
long event)
} else {
info = kzalloc(size, GFP_KERNEL);
if (!info) {
- pr_warn("Out of memory when allocating notify_info "
- "for %s.\n", pci_name(dev));
if (dmar_dev_scope_status == 0)
dmar_dev_scope_status = -ENOMEM;
return NULL;
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] Exynos-IOMMU: Delete an error message for a failed memory allocation in exynos_iommu_init()
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 20 Jan 2018 14:28:13 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/exynos-iommu.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 79c45650f8de..456a88483089 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1370,8 +1370,6 @@ static int __init exynos_iommu_init(void)
zero_lv2_table = kmem_cache_zalloc(lv2table_kmem_cache, GFP_KERNEL);
if (zero_lv2_table == NULL) {
- pr_err("%s: Failed to allocate zero level2 page table\n",
- __func__);
ret = -ENOMEM;
goto err_zero_lv2;
}
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] Intel-IOMMU: Delete an error message for a failed memory allocation in init_dmars()
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 20 Jan 2018 13:51:49 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/intel-iommu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4a2de34895ec..f4ba6bdf7863 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3258,7 +3258,6 @@ static int __init init_dmars(void)
g_iommus = kcalloc(g_num_of_iommus, sizeof(struct intel_iommu *),
GFP_KERNEL);
if (!g_iommus) {
- pr_err("Allocating global iommu array failed\n");
ret = -ENOMEM;
goto error;
}
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] IPMMU-VMSA: Delete an error message for a failed memory allocation in ipmmu_probe()
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Fri, 19 Jan 2018 22:22:38 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/ipmmu-vmsa.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 8dce3a9de9d8..1f78d60f2029 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -949,10 +949,8 @@ static int ipmmu_probe(struct platform_device *pdev)
int ret;
mmu = devm_kzalloc(>dev, sizeof(*mmu), GFP_KERNEL);
- if (!mmu) {
- dev_err(>dev, "cannot allocate device data\n");
+ if (!mmu)
return -ENOMEM;
- }
mmu->dev = >dev;
mmu->num_utlbs = 32;
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] IOMMU-Tegra: gart: Delete two error messages for a failed memory allocation in tegra_gart_probe()
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Fri, 19 Jan 2018 21:44:31 +0100
Omit extra messages for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
drivers/iommu/tegra-gart.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c
index b62f790ad1ba..1fee3a956832 100644
--- a/drivers/iommu/tegra-gart.c
+++ b/drivers/iommu/tegra-gart.c
@@ -419,10 +419,8 @@ static int tegra_gart_probe(struct platform_device *pdev)
}
gart = devm_kzalloc(dev, sizeof(*gart), GFP_KERNEL);
- if (!gart) {
- dev_err(dev, "failed to allocate gart_device\n");
+ if (!gart)
return -ENOMEM;
- }
gart_regs = devm_ioremap(dev, res->start, resource_size(res));
if (!gart_regs) {
@@ -455,10 +453,8 @@ static int tegra_gart_probe(struct platform_device *pdev)
gart->page_count = (resource_size(res_remap) >> GART_PAGE_SHIFT);
gart->savedata = vmalloc(sizeof(u32) * gart->page_count);
- if (!gart->savedata) {
- dev_err(dev, "failed to allocate context save area\n");
+ if (!gart->savedata)
return -ENOMEM;
- }
platform_set_drvdata(pdev, gart);
do_gart_setup(gart, NULL);
--
2.15.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] lib: Improve a size determination in seven functions
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 7 Oct 2017 19:19:45 +0200
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
lib/dma-debug.c | 2 +-
lib/flex_array.c| 4 ++--
lib/genalloc.c | 2 +-
lib/lru_cache.c | 2 +-
lib/reed_solomon/reed_solomon.c | 2 +-
lib/test_kmod.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index ea4cc3dde4f1..8b5c8fc76f8a 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -993,7 +993,7 @@ void dma_debug_add_bus(struct bus_type *bus)
if (dma_debug_disabled())
return;
- nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
+ nb = kzalloc(sizeof(*nb), GFP_KERNEL);
if (nb == NULL) {
pr_err("dma_debug_add_bus: out of memory\n");
return;
diff --git a/lib/flex_array.c b/lib/flex_array.c
index 2eed22fa507c..ebe0f366b6cb 100644
--- a/lib/flex_array.c
+++ b/lib/flex_array.c
@@ -102,7 +102,7 @@ struct flex_array *flex_array_alloc(int element_size,
unsigned int total,
/* max_size will end up 0 if element_size > PAGE_SIZE */
if (total > max_size)
return NULL;
- ret = kzalloc(sizeof(struct flex_array), flags);
+ ret = kzalloc(sizeof(*ret), flags);
if (!ret)
return NULL;
ret->element_size = element_size;
@@ -167,7 +167,7 @@ __fa_get_part(struct flex_array *fa, int part_nr, gfp_t
flags)
{
struct flex_array_part *part = fa->parts[part_nr];
if (!part) {
- part = kmalloc(sizeof(struct flex_array_part), flags);
+ part = kmalloc(sizeof(*part), flags);
if (!part)
return NULL;
if (!(flags & __GFP_ZERO))
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 144fe6b1a03e..696cf1236b6c 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -153,7 +153,7 @@ struct gen_pool *gen_pool_create(int min_alloc_order, int
nid)
{
struct gen_pool *pool;
- pool = kmalloc_node(sizeof(struct gen_pool), GFP_KERNEL, nid);
+ pool = kmalloc_node(sizeof(*pool), GFP_KERNEL, nid);
if (pool != NULL) {
spin_lock_init(>lock);
INIT_LIST_HEAD(>chunks);
diff --git a/lib/lru_cache.c b/lib/lru_cache.c
index 28ba40b99337..4882ed22a8ce 100644
--- a/lib/lru_cache.c
+++ b/lib/lru_cache.c
@@ -116,7 +116,7 @@ struct lru_cache *lc_create(const char *name, struct
kmem_cache *cache,
if (e_count > LC_MAX_ACTIVE)
return NULL;
- slot = kcalloc(e_count, sizeof(struct hlist_head), GFP_KERNEL);
+ slot = kcalloc(e_count, sizeof(*slot), GFP_KERNEL);
if (!slot)
goto out_fail;
element = kzalloc(e_count * sizeof(struct lc_element *), GFP_KERNEL);
diff --git a/lib/reed_solomon/reed_solomon.c b/lib/reed_solomon/reed_solomon.c
index 06d04cfa9339..2d0ec9f84322 100644
--- a/lib/reed_solomon/reed_solomon.c
+++ b/lib/reed_solomon/reed_solomon.c
@@ -70,7 +70,7 @@ static struct rs_control *rs_init(int symsize, int gfpoly,
int (*gffunc)(int),
int i, j, sr, root, iprim;
/* Allocate the control structure */
- rs = kmalloc(sizeof (struct rs_control), GFP_KERNEL);
+ rs = kmalloc(sizeof(*rs), GFP_KERNEL);
if (rs == NULL)
return NULL;
diff --git a/lib/test_kmod.c b/lib/test_kmod.c
index 337f408b4de6..57f3337f2482 100644
--- a/lib/test_kmod.c
+++ b/lib/test_kmod.c
@@ -1086,7 +1086,7 @@ static struct kmod_test_device *alloc_test_dev_kmod(int
idx)
struct kmod_test_device *test_dev;
struct miscdevice *misc_dev;
- test_dev = vzalloc(sizeof(struct kmod_test_device));
+ test_dev = vzalloc(sizeof(*test_dev));
if (!test_dev)
goto err_out;
--
2.14.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] lib/test: Delete five error messages for a failed memory allocation
From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Sat, 7 Oct 2017 17:34:23 +0200
Omit extra messages for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
---
lib/test_kasan.c | 5 ++---
lib/test_kmod.c | 8 ++--
lib/test_list_sort.c | 9 +++--
3 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index a25c9763fce1..ef1a3ac1397e 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -353,10 +353,9 @@ static noinline void __init
memcg_accounted_kmem_cache(void)
*/
for (i = 0; i < 5; i++) {
p = kmem_cache_alloc(cache, GFP_KERNEL);
- if (!p) {
- pr_err("Allocation failed\n");
+ if (!p)
goto free_cache;
- }
+
kmem_cache_free(cache, p);
msleep(100);
}
diff --git a/lib/test_kmod.c b/lib/test_kmod.c
index fba78d25e825..337f408b4de6 100644
--- a/lib/test_kmod.c
+++ b/lib/test_kmod.c
@@ -783,10 +783,8 @@ static int kmod_config_sync_info(struct kmod_test_device
*test_dev)
free_test_dev_info(test_dev);
test_dev->info = vzalloc(config->num_threads *
sizeof(struct kmod_test_device_info));
- if (!test_dev->info) {
- dev_err(test_dev->dev, "Cannot alloc test_dev info\n");
+ if (!test_dev->info)
return -ENOMEM;
- }
return 0;
}
@@ -1089,10 +1087,8 @@ static struct kmod_test_device *alloc_test_dev_kmod(int
idx)
struct miscdevice *misc_dev;
test_dev = vzalloc(sizeof(struct kmod_test_device));
- if (!test_dev) {
- pr_err("Cannot alloc test_dev\n");
+ if (!test_dev)
goto err_out;
- }
mutex_init(_dev->config_mutex);
mutex_init(_dev->trigger_mutex);
diff --git a/lib/test_list_sort.c b/lib/test_list_sort.c
index 28e817387b04..5474f3f3e41d 100644
--- a/lib/test_list_sort.c
+++ b/lib/test_list_sort.c
@@ -76,17 +76,14 @@ static int __init list_sort_test(void)
pr_debug("start testing list_sort()\n");
elts = kcalloc(TEST_LIST_LEN, sizeof(*elts), GFP_KERNEL);
- if (!elts) {
- pr_err("error: cannot allocate memory\n");
+ if (!elts)
return err;
- }
for (i = 0; i < TEST_LIST_LEN; i++) {
el = kmalloc(sizeof(*el), GFP_KERNEL);
- if (!el) {
- pr_err("error: cannot allocate memory\n");
+ if (!el)
goto exit;
- }
+
/* force some equivalencies */
el->value = prandom_u32() % (TEST_LIST_LEN / 3);
el->serial = i;
--
2.14.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] lib: Adjustments for 11 function implementations
From: Markus Elfring <elfr...@users.sourceforge.net> Date: Sat, 7 Oct 2017 19:39:29 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): test: Delete five error messages for a failed memory allocation Improve a size determination in seven functions lib/dma-debug.c | 2 +- lib/flex_array.c| 4 ++-- lib/genalloc.c | 2 +- lib/lru_cache.c | 2 +- lib/reed_solomon/reed_solomon.c | 2 +- lib/test_kasan.c| 5 ++--- lib/test_kmod.c | 10 +++--- lib/test_list_sort.c| 9 +++-- 8 files changed, 14 insertions(+), 22 deletions(-) -- 2.14.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 00/39] drop null test before destroy functions
> Recent commits to kernel/git/torvalds/linux.git have made the following > functions able to tolerate NULL arguments: > > kmem_cache_destroy (commit 3942d29918522) > mempool_destroy (commit 4e3ca3e033d1) > dma_pool_destroy (commit 44d7175da6ea) How do you think about to extend an other SmPL script? Related topic: scripts/coccinelle/free: Delete NULL test before freeing functions https://systeme.lip6.fr/pipermail/cocci/2015-May/001960.html https://www.mail-archive.com/cocci@systeme.lip6.fr/msg01855.html > If these changes are OK, I will address the remainder later. Would anybody like to reuse my general SmPL approach for similar source code clean-up? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] IOMMU-Tegra: gart: Delete an unnecessary check before the function call vfree
From: Markus Elfring elfr...@users.sourceforge.net Date: Thu, 5 Feb 2015 17:54:16 +0100 The vfree() function performs also input parameter validation. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring elfr...@users.sourceforge.net --- drivers/iommu/tegra-gart.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c index f722a0c..07a795a 100644 --- a/drivers/iommu/tegra-gart.c +++ b/drivers/iommu/tegra-gart.c @@ -404,8 +404,7 @@ static int tegra_gart_remove(struct platform_device *pdev) struct gart_device *gart = platform_get_drvdata(pdev); writel(0, gart-regs + GART_CONFIG); - if (gart-savedata) - vfree(gart-savedata); + vfree(gart-savedata); gart_handle = NULL; return 0; } Would you like to integrate this update suggestion into another source code repository? Regards, Markus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu: Delete an unnecessary check before the function call free_io_pgtable_ops
From: Markus Elfring elfr...@users.sourceforge.net
Date: Sun, 28 Jun 2015 15:55:11 +0200
The free_io_pgtable_ops() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfr...@users.sourceforge.net
---
drivers/iommu/arm-smmu-v3.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f141301..8e9ec81 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1389,8 +1389,7 @@ static void arm_smmu_domain_free(struct iommu_domain
*domain)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain-smmu;
- if (smmu_domain-pgtbl_ops)
- free_io_pgtable_ops(smmu_domain-pgtbl_ops);
+ free_io_pgtable_ops(smmu_domain-pgtbl_ops);
/* Free the CD and ASID, if we allocated them */
if (smmu_domain-stage == ARM_SMMU_DOMAIN_S1) {
--
2.4.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] IOMMU-Tegra: gart: Delete an unnecessary check before the function call vfree
From: Markus Elfring elfr...@users.sourceforge.net Date: Thu, 5 Feb 2015 17:54:16 +0100 The vfree() function performs also input parameter validation. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring elfr...@users.sourceforge.net --- drivers/iommu/tegra-gart.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/iommu/tegra-gart.c b/drivers/iommu/tegra-gart.c index f722a0c..07a795a 100644 --- a/drivers/iommu/tegra-gart.c +++ b/drivers/iommu/tegra-gart.c @@ -404,8 +404,7 @@ static int tegra_gart_remove(struct platform_device *pdev) struct gart_device *gart = platform_get_drvdata(pdev); writel(0, gart-regs + GART_CONFIG); - if (gart-savedata) - vfree(gart-savedata); + vfree(gart-savedata); gart_handle = NULL; return 0; } -- 2.2.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] IOMMU-MSM: Deletion of unnecessary checks before the function call clk_disable
If you are convinced that dropping the null tests is a good idea, then you
can submit the patch that makes the change to the relevant maintainers and
mailing lists.
From af73fb59d5d4b2c289fb236d0752522b6b38 Mon Sep 17 00:00:00 2001
From: Markus Elfring elfr...@users.sourceforge.net
Date: Wed, 22 Oct 2014 19:39:21 +0200
Subject: [PATCH] IOMMU-MSM: Deletion of unnecessary checks before the function
call clk_disable
A semantic patch approach was proposed with the subject [PATCH with
Coccinelle?] Deletion of unnecessary checks before specific function calls
on 2014-03-05.
https://lkml.org/lkml/2014/3/5/344
http://article.gmane.org/gmane.comp.version-control.coccinelle/3513/
This patch pattern application was repeated with the help of the software
Coccinelle 1.0.0-rc22 on the source files for Linux 3.17.1. An extract
of the automatically generated update suggestions is shown here.
It was determined that the affected source code places call functions
which perform input parameter validation already. It is therefore not
needed that a similar safety check is repeated at the call site.
Signed-off-by: Markus Elfring elfr...@users.sourceforge.net
---
drivers/iommu/msm_iommu.c | 3 +--
drivers/iommu/msm_iommu_dev.c | 6 ++
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 6e3dcc28..3e4d888 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -73,8 +73,7 @@ fail:
static void __disable_clocks(struct msm_iommu_drvdata *drvdata)
{
- if (drvdata-clk)
- clk_disable(drvdata-clk);
+ clk_disable(drvdata-clk);
clk_disable(drvdata-pclk);
}
diff --git a/drivers/iommu/msm_iommu_dev.c b/drivers/iommu/msm_iommu_dev.c
index 61def7cb..9574d21 100644
--- a/drivers/iommu/msm_iommu_dev.c
+++ b/drivers/iommu/msm_iommu_dev.c
@@ -224,8 +224,7 @@ static int msm_iommu_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, drvdata);
- if (iommu_clk)
- clk_disable(iommu_clk);
+ clk_disable(iommu_clk);
clk_disable(iommu_pclk);
@@ -323,8 +322,7 @@ static int msm_iommu_ctx_probe(struct platform_device *pdev)
SET_NSCFG(drvdata-base, mid, 3);
}
- if (drvdata-clk)
- clk_disable(drvdata-clk);
+ clk_disable(drvdata-clk);
clk_disable(drvdata-pclk);
dev_info(pdev-dev, context %s using bank %d\n, c-name, c-num);
--
2.1.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
