* [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found
@ 2015-09-29 7:26 Baoquan He
2015-09-29 7:26 ` [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir Baoquan He
2015-09-29 13:50 ` [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Joerg Roedel
0 siblings, 2 replies; 5+ messages in thread
From: Baoquan He @ 2015-09-29 7:26 UTC (permalink / raw)
To: joro; +Cc: iommu, linux-kernel, Baoquan He
No need to continue the loop after it has been found.
Signed-off-by: Baoquan He <bhe@redhat.com>
---
drivers/iommu/intel_irq_remapping.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 9ec4e0d..37b93f5 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -841,8 +841,10 @@ static int ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope,
if (ir_ioapic[count].iommu == iommu &&
ir_ioapic[count].id == scope->enumeration_id)
return 0;
- else if (ir_ioapic[count].iommu == NULL && free == -1)
+ else if (ir_ioapic[count].iommu == NULL && free == -1) {
free = count;
+ break;
+ }
}
if (free == -1) {
pr_warn("Exceeded Max IO APICS\n");
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir
2015-09-29 7:26 [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Baoquan He
@ 2015-09-29 7:26 ` Baoquan He
2015-09-29 13:55 ` Joerg Roedel
2015-09-29 13:50 ` [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Joerg Roedel
1 sibling, 1 reply; 5+ messages in thread
From: Baoquan He @ 2015-09-29 7:26 UTC (permalink / raw)
To: joro; +Cc: iommu, linux-kernel, Baoquan He
Adjust the return value of parse_ioapics_under_ir as "-1" representing
failure and "0" representing succcess. Just make it consistent with other
function implementation, and we can judge if calling is successfull by
if (!parse_ioapics_under_ir()) style.
Signed-off-by: Baoquan He <bhe@redhat.com>
---
drivers/iommu/intel_irq_remapping.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 37b93f5..0f441b7 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -672,7 +672,7 @@ static int __init intel_prepare_irq_remapping(void)
if (!dmar_ir_support())
return -ENODEV;
- if (parse_ioapics_under_ir() != 1) {
+ if (!parse_ioapics_under_ir()) {
pr_info("Not enabling interrupt remapping\n");
goto error;
}
@@ -918,7 +918,7 @@ static int __init parse_ioapics_under_ir(void)
}
if (!ir_supported)
- return 0;
+ return -1;
for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++) {
int ioapic_id = mpc_ioapic_id(ioapic_idx);
@@ -930,7 +930,7 @@ static int __init parse_ioapics_under_ir(void)
}
}
- return 1;
+ return 0;
}
static int __init ir_dev_scope_init(void)
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir
2015-09-29 7:26 ` [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir Baoquan He
@ 2015-09-29 13:55 ` Joerg Roedel
0 siblings, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2015-09-29 13:55 UTC (permalink / raw)
To: Baoquan He; +Cc: iommu, linux-kernel
On Tue, Sep 29, 2015 at 03:26:09PM +0800, Baoquan He wrote:
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index 37b93f5..0f441b7 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -672,7 +672,7 @@ static int __init intel_prepare_irq_remapping(void)
> if (!dmar_ir_support())
> return -ENODEV;
>
> - if (parse_ioapics_under_ir() != 1) {
> + if (!parse_ioapics_under_ir()) {
> pr_info("Not enabling interrupt remapping\n");
> goto error;
> }
> @@ -918,7 +918,7 @@ static int __init parse_ioapics_under_ir(void)
> }
>
> if (!ir_supported)
> - return 0;
> + return -1;
>
> for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++) {
> int ioapic_id = mpc_ioapic_id(ioapic_idx);
> @@ -930,7 +930,7 @@ static int __init parse_ioapics_under_ir(void)
> }
> }
>
> - return 1;
> + return 0;
Looks good, but -ENODEV is probably a better return value than -1.
Joerg
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found
2015-09-29 7:26 [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Baoquan He
2015-09-29 7:26 ` [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir Baoquan He
@ 2015-09-29 13:50 ` Joerg Roedel
2015-09-29 15:04 ` Baoquan He
1 sibling, 1 reply; 5+ messages in thread
From: Joerg Roedel @ 2015-09-29 13:50 UTC (permalink / raw)
To: Baoquan He; +Cc: iommu, linux-kernel
On Tue, Sep 29, 2015 at 03:26:08PM +0800, Baoquan He wrote:
> No need to continue the loop after it has been found.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> drivers/iommu/intel_irq_remapping.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index 9ec4e0d..37b93f5 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -841,8 +841,10 @@ static int ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope,
> if (ir_ioapic[count].iommu == iommu &&
> ir_ioapic[count].id == scope->enumeration_id)
> return 0;
> - else if (ir_ioapic[count].iommu == NULL && free == -1)
> + else if (ir_ioapic[count].iommu == NULL && free == -1) {
> free = count;
> + break;
> + }
> }
The purpose of the loop is also to avoid duplicate entries, so we need
to search it till the end, no?
Joerg
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found
2015-09-29 13:50 ` [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Joerg Roedel
@ 2015-09-29 15:04 ` Baoquan He
0 siblings, 0 replies; 5+ messages in thread
From: Baoquan He @ 2015-09-29 15:04 UTC (permalink / raw)
To: Joerg Roedel; +Cc: iommu, linux-kernel
On 09/29/15 at 03:50pm, Joerg Roedel wrote:
> On Tue, Sep 29, 2015 at 03:26:08PM +0800, Baoquan He wrote:
> > No need to continue the loop after it has been found.
> >
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > ---
> > drivers/iommu/intel_irq_remapping.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> > index 9ec4e0d..37b93f5 100644
> > --- a/drivers/iommu/intel_irq_remapping.c
> > +++ b/drivers/iommu/intel_irq_remapping.c
> > @@ -841,8 +841,10 @@ static int ir_parse_one_ioapic_scope(struct acpi_dmar_device_scope *scope, > > if (ir_ioapic[count].iommu == iommu &&
> > ir_ioapic[count].id == scope->enumeration_id)
> > return 0;
> > - else if (ir_ioapic[count].iommu == NULL && free == -1)
> > + else if (ir_ioapic[count].iommu == NULL && free == -1) {
> > free = count;
> > + break;
> > + }
> > }
>
> The purpose of the loop is also to avoid duplicate entries, so we need
> to search it till the end, no?
Ah, yes, you are right, it's on purpose. So I will drop this one
and update the 2/2 only with your suggestion.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-09-29 15:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-29 7:26 [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Baoquan He
2015-09-29 7:26 ` [PATCH 2/2] iommu/vt-d: Adjsut the return value of the parse_ioapics_under_ir Baoquan He
2015-09-29 13:55 ` Joerg Roedel
2015-09-29 13:50 ` [PATCH 1/2] iommu/vt-d: break the for loop if an empty ir_ioapic entry found Joerg Roedel
2015-09-29 15:04 ` Baoquan He
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox