* [PATCH] irq: Remove irqaction.free_on_release
@ 2012-10-08 13:09 Andrew Cooper
2012-10-09 11:44 ` Keir Fraser
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2012-10-08 13:09 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Jan Beulich, Stefano Stabellini
It is always set to 1, and only checked on some of the codepaths which free an
irqaction.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
--
This patch does touch common code as well as x86 and arm architectures,
so probably needs quite a few acks.
diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/gic.c
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -373,7 +373,7 @@ void __init release_irq(unsigned int irq
/* Wait to make sure it's not being used on another CPU */
do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
- if (action && action->free_on_release)
+ if ( action )
xfree(action);
}
diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/irq.c
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -97,7 +97,6 @@ int __init request_irq(unsigned int irq,
action->handler = handler;
action->name = devname;
action->dev_id = dev_id;
- action->free_on_release = 1;
retval = setup_irq(irq, action);
if (retval)
diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -952,7 +952,6 @@ int __init request_irq(unsigned int irq,
action->handler = handler;
action->name = devname;
action->dev_id = dev_id;
- action->free_on_release = 1;
retval = setup_irq(irq, action);
if (retval)
@@ -979,7 +978,7 @@ void __init release_irq(unsigned int irq
/* Wait to make sure it's not being used on another CPU */
do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
- if (action && action->free_on_release)
+ if ( action )
xfree(action);
}
diff -r 5fbdbf585f5f -r 37f1afbec80b xen/include/xen/irq.h
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -13,7 +13,6 @@ struct irqaction {
void (*handler)(int, void *, struct cpu_user_regs *);
const char *name;
void *dev_id;
- bool_t free_on_release;
};
/*
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] irq: Remove irqaction.free_on_release
2012-10-08 13:09 [PATCH] irq: Remove irqaction.free_on_release Andrew Cooper
@ 2012-10-09 11:44 ` Keir Fraser
2012-10-09 14:38 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Keir Fraser @ 2012-10-09 11:44 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: Ian Campbell, Jan Beulich, Stefano Stabellini
But reasoning behind c/s 20153, which introduced it, still applies? Callers
using setup_irq() don't want their passed-in irqaction to be xfree()ed?
-- Keir
On 08/10/2012 14:09, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:
> It is always set to 1, and only checked on some of the codepaths which free an
> irqaction.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --
> This patch does touch common code as well as x86 and arm architectures,
> so probably needs quite a few acks.
>
> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/gic.c
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -373,7 +373,7 @@ void __init release_irq(unsigned int irq
> /* Wait to make sure it's not being used on another CPU */
> do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>
> - if (action && action->free_on_release)
> + if ( action )
> xfree(action);
> }
>
> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/irq.c
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -97,7 +97,6 @@ int __init request_irq(unsigned int irq,
> action->handler = handler;
> action->name = devname;
> action->dev_id = dev_id;
> - action->free_on_release = 1;
>
> retval = setup_irq(irq, action);
> if (retval)
> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -952,7 +952,6 @@ int __init request_irq(unsigned int irq,
> action->handler = handler;
> action->name = devname;
> action->dev_id = dev_id;
> - action->free_on_release = 1;
>
> retval = setup_irq(irq, action);
> if (retval)
> @@ -979,7 +978,7 @@ void __init release_irq(unsigned int irq
> /* Wait to make sure it's not being used on another CPU */
> do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>
> - if (action && action->free_on_release)
> + if ( action )
> xfree(action);
> }
>
> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/include/xen/irq.h
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -13,7 +13,6 @@ struct irqaction {
> void (*handler)(int, void *, struct cpu_user_regs *);
> const char *name;
> void *dev_id;
> - bool_t free_on_release;
> };
>
> /*
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] irq: Remove irqaction.free_on_release
2012-10-09 11:44 ` Keir Fraser
@ 2012-10-09 14:38 ` Andrew Cooper
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2012-10-09 14:38 UTC (permalink / raw)
To: Keir Fraser
Cc: Stefano Stabellini, Ian Campbell, Jan Beulich,
xen-devel@lists.xen.org
On 09/10/12 12:44, Keir Fraser wrote:
> But reasoning behind c/s 20153, which introduced it, still applies? Callers
> using setup_irq() don't want their passed-in irqaction to be xfree()ed?
>
> -- Keir
Yikes - yes it does. My original reading of the code was that it was
being set, hence the patch, but upon closer reading I am wrong.
As for grep'ing, it is only ever implicitly set to 0 by compound struct
initialisation with { 0 }
Furthermore, there is now a case where an ns16550 uart->irq can be set
higher than nr_irqs_gsi, although it is unclear whether this will
actually cause a problem and hit the unconditional xfree(action) in
dynamic_irq_cleanip(), which is only protected by BUG()'ing if the irq
index is within the gsi range.
~Andrew
>
> On 08/10/2012 14:09, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:
>
>> It is always set to 1, and only checked on some of the codepaths which free an
>> irqaction.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> --
>> This patch does touch common code as well as x86 and arm architectures,
>> so probably needs quite a few acks.
>>
>> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/gic.c
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -373,7 +373,7 @@ void __init release_irq(unsigned int irq
>> /* Wait to make sure it's not being used on another CPU */
>> do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>>
>> - if (action && action->free_on_release)
>> + if ( action )
>> xfree(action);
>> }
>>
>> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/arm/irq.c
>> --- a/xen/arch/arm/irq.c
>> +++ b/xen/arch/arm/irq.c
>> @@ -97,7 +97,6 @@ int __init request_irq(unsigned int irq,
>> action->handler = handler;
>> action->name = devname;
>> action->dev_id = dev_id;
>> - action->free_on_release = 1;
>>
>> retval = setup_irq(irq, action);
>> if (retval)
>> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/arch/x86/irq.c
>> --- a/xen/arch/x86/irq.c
>> +++ b/xen/arch/x86/irq.c
>> @@ -952,7 +952,6 @@ int __init request_irq(unsigned int irq,
>> action->handler = handler;
>> action->name = devname;
>> action->dev_id = dev_id;
>> - action->free_on_release = 1;
>>
>> retval = setup_irq(irq, action);
>> if (retval)
>> @@ -979,7 +978,7 @@ void __init release_irq(unsigned int irq
>> /* Wait to make sure it's not being used on another CPU */
>> do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>>
>> - if (action && action->free_on_release)
>> + if ( action )
>> xfree(action);
>> }
>>
>> diff -r 5fbdbf585f5f -r 37f1afbec80b xen/include/xen/irq.h
>> --- a/xen/include/xen/irq.h
>> +++ b/xen/include/xen/irq.h
>> @@ -13,7 +13,6 @@ struct irqaction {
>> void (*handler)(int, void *, struct cpu_user_regs *);
>> const char *name;
>> void *dev_id;
>> - bool_t free_on_release;
>> };
>>
>> /*
>
--
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-09 14:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-08 13:09 [PATCH] irq: Remove irqaction.free_on_release Andrew Cooper
2012-10-09 11:44 ` Keir Fraser
2012-10-09 14:38 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).