* [PATCH] iommu/vt-d: Unlock on error paths
@ 2020-03-12 11:37 Dan Carpenter
2020-03-12 12:02 ` Lu Baolu
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2020-03-12 11:37 UTC (permalink / raw)
To: David Woodhouse, Megha Dey; +Cc: kernel-janitors, iommu
There were a couple places where we need to unlock before returning.
Fixes: 91391b919e19 ("iommu/vt-d: Populate debugfs if IOMMUs are detected")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/iommu/intel-iommu-debugfs.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c
index 8d24c4d85cc2..6a495b103972 100644
--- a/drivers/iommu/intel-iommu-debugfs.c
+++ b/drivers/iommu/intel-iommu-debugfs.c
@@ -289,11 +289,12 @@ static int dmar_translation_struct_show(struct seq_file *m, void *unused)
sts = dmar_readl(iommu->reg + DMAR_GSTS_REG);
if (!(sts & DMA_GSTS_TES)) {
seq_puts(m, "DMA Remapping is not enabled\n");
- return 0;
+ goto unlock;
}
root_tbl_walk(m, iommu);
seq_putc(m, '\n');
}
+unlock:
rcu_read_unlock();
return 0;
@@ -444,7 +445,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused)
sts = dmar_readl(iommu->reg + DMAR_GSTS_REG);
if (!(sts & DMA_GSTS_IRES)) {
seq_puts(m, "Interrupt Remapping is not enabled\n");
- return 0;
+ goto unlock;
}
if (iommu->ir_table) {
@@ -475,6 +476,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused)
}
seq_putc(m, '\n');
}
+unlock:
rcu_read_unlock();
return 0;
--
2.20.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iommu/vt-d: Unlock on error paths 2020-03-12 11:37 [PATCH] iommu/vt-d: Unlock on error paths Dan Carpenter @ 2020-03-12 12:02 ` Lu Baolu 2020-03-12 12:55 ` Dan Carpenter 0 siblings, 1 reply; 3+ messages in thread From: Lu Baolu @ 2020-03-12 12:02 UTC (permalink / raw) To: Dan Carpenter, David Woodhouse, Megha Dey; +Cc: kernel-janitors, iommu On 2020/3/12 19:37, Dan Carpenter wrote: > There were a couple places where we need to unlock before returning. > > Fixes: 91391b919e19 ("iommu/vt-d: Populate debugfs if IOMMUs are detected") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/iommu/intel-iommu-debugfs.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c > index 8d24c4d85cc2..6a495b103972 100644 > --- a/drivers/iommu/intel-iommu-debugfs.c > +++ b/drivers/iommu/intel-iommu-debugfs.c > @@ -289,11 +289,12 @@ static int dmar_translation_struct_show(struct seq_file *m, void *unused) > sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); > if (!(sts & DMA_GSTS_TES)) { > seq_puts(m, "DMA Remapping is not enabled\n"); > - return 0; > + goto unlock; > } > root_tbl_walk(m, iommu); > seq_putc(m, '\n'); > } > +unlock: > rcu_read_unlock(); > > return 0; > @@ -444,7 +445,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) > sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); > if (!(sts & DMA_GSTS_IRES)) { > seq_puts(m, "Interrupt Remapping is not enabled\n"); > - return 0; > + goto unlock; > } > > if (iommu->ir_table) { > @@ -475,6 +476,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) > } > seq_putc(m, '\n'); > } > +unlock: > rcu_read_unlock(); > > return 0; > Thanks a lot for the catch. I think it could be further cleanup. How about below changes? index 8d24c4d85cc2..2583a6743dd0 100644 --- a/drivers/iommu/intel-iommu-debugfs.c +++ b/drivers/iommu/intel-iommu-debugfs.c @@ -288,8 +288,9 @@ static int dmar_translation_struct_show(struct seq_file *m, void *unused) for_each_active_iommu(iommu, drhd) { sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); if (!(sts & DMA_GSTS_TES)) { - seq_puts(m, "DMA Remapping is not enabled\n"); - return 0; + seq_printf(m, "DMA Remapping is not enabled on %s\n", + iommu->name); + continue; } root_tbl_walk(m, iommu); seq_putc(m, '\n'); @@ -431,7 +432,6 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) struct dmar_drhd_unit *drhd; struct intel_iommu *iommu; u64 irta; - u32 sts; rcu_read_lock(); for_each_active_iommu(iommu, drhd) { @@ -441,12 +441,6 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) seq_printf(m, "Remapped Interrupt supported on IOMMU: %s\n", iommu->name); - sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); - if (!(sts & DMA_GSTS_IRES)) { - seq_puts(m, "Interrupt Remapping is not enabled\n"); - return 0; - } - if (iommu->ir_table) { irta = virt_to_phys(iommu->ir_table->base); seq_printf(m, " IR table address:%llx\n", irta); Best regards, baolu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iommu/vt-d: Unlock on error paths 2020-03-12 12:02 ` Lu Baolu @ 2020-03-12 12:55 ` Dan Carpenter 0 siblings, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2020-03-12 12:55 UTC (permalink / raw) To: Lu Baolu; +Cc: kernel-janitors, Megha Dey, David Woodhouse, iommu On Thu, Mar 12, 2020 at 08:02:41PM +0800, Lu Baolu wrote: > On 2020/3/12 19:37, Dan Carpenter wrote: > > There were a couple places where we need to unlock before returning. > > > > Fixes: 91391b919e19 ("iommu/vt-d: Populate debugfs if IOMMUs are detected") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/iommu/intel-iommu-debugfs.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iommu/intel-iommu-debugfs.c b/drivers/iommu/intel-iommu-debugfs.c > > index 8d24c4d85cc2..6a495b103972 100644 > > --- a/drivers/iommu/intel-iommu-debugfs.c > > +++ b/drivers/iommu/intel-iommu-debugfs.c > > @@ -289,11 +289,12 @@ static int dmar_translation_struct_show(struct seq_file *m, void *unused) > > sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); > > if (!(sts & DMA_GSTS_TES)) { > > seq_puts(m, "DMA Remapping is not enabled\n"); > > - return 0; > > + goto unlock; > > } > > root_tbl_walk(m, iommu); > > seq_putc(m, '\n'); > > } > > +unlock: > > rcu_read_unlock(); > > return 0; > > @@ -444,7 +445,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) > > sts = dmar_readl(iommu->reg + DMAR_GSTS_REG); > > if (!(sts & DMA_GSTS_IRES)) { > > seq_puts(m, "Interrupt Remapping is not enabled\n"); > > - return 0; > > + goto unlock; > > } > > if (iommu->ir_table) { > > @@ -475,6 +476,7 @@ static int ir_translation_struct_show(struct seq_file *m, void *unused) > > } > > seq_putc(m, '\n'); > > } > > +unlock: > > rcu_read_unlock(); > > return 0; > > > > Thanks a lot for the catch. I think it could be further cleanup. How > about below changes? Obviously that solves the issues with forgetting to drop the lock but I'm not qualified to comment on the rest. (And I can't really review it anyway because the patch was damaged in sending the email). regards, dan carepnter _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-12 12:56 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-12 11:37 [PATCH] iommu/vt-d: Unlock on error paths Dan Carpenter 2020-03-12 12:02 ` Lu Baolu 2020-03-12 12:55 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox