From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: David Vrabel <david.vrabel@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Jan Beulich <jbeulich@suse.com>,
xen-devel@lists.xen.org
Subject: Re: [PATCH 08/13] xen/events: allow setup of irq_info to fail
Date: Tue, 24 Sep 2013 10:47:03 -0400 [thread overview]
Message-ID: <20130924144703.GG4712@phenom.dumpdata.com> (raw)
In-Reply-To: <1379091601-30358-9-git-send-email-david.vrabel@citrix.com>
On Fri, Sep 13, 2013 at 05:59:56PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> The FIFO-based event ABI requires additional setup of newly bound
> events (it may need to expand the event array) and this setup may
> fail.
>
> xen_irq_info_common_init() is a useful place to put this setup so
> allow this call to fail. This call and the other similar calls are
> renamed to be *_setup() to reflect that they may now fail.
>
> This failure can only occur with new event channels not on rebind.
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/xen/events/events.c | 156 +++++++++++++++++++++++++------------------
> 1 files changed, 91 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/xen/events/events.c b/drivers/xen/events/events.c
> index 8ecde63..8f55a49 100644
> --- a/drivers/xen/events/events.c
> +++ b/drivers/xen/events/events.c
> @@ -99,7 +99,7 @@ struct irq_info *info_for_irq(unsigned irq)
> }
>
> /* Constructors for packed IRQ information. */
> -static void xen_irq_info_common_init(struct irq_info *info,
> +static int xen_irq_info_common_setup(struct irq_info *info,
> unsigned irq,
> enum xen_irq_type type,
> unsigned short evtchn,
> @@ -116,45 +116,47 @@ static void xen_irq_info_common_init(struct irq_info *info,
> evtchn_to_irq[evtchn] = irq;
>
> irq_clear_status_flags(irq, IRQ_NOREQUEST|IRQ_NOAUTOEN);
> +
> + return 0;
> }
>
> -static void xen_irq_info_evtchn_init(unsigned irq,
> +static int xen_irq_info_evtchn_setup(unsigned irq,
> unsigned short evtchn)
> {
> struct irq_info *info = info_for_irq(irq);
>
> - xen_irq_info_common_init(info, irq, IRQT_EVTCHN, evtchn, 0);
> + return xen_irq_info_common_setup(info, irq, IRQT_EVTCHN, evtchn, 0);
> }
>
> -static void xen_irq_info_ipi_init(unsigned cpu,
> +static int xen_irq_info_ipi_setup(unsigned cpu,
> unsigned irq,
> unsigned short evtchn,
> enum ipi_vector ipi)
> {
> struct irq_info *info = info_for_irq(irq);
>
> - xen_irq_info_common_init(info, irq, IRQT_IPI, evtchn, 0);
> -
> info->u.ipi = ipi;
>
> per_cpu(ipi_to_irq, cpu)[ipi] = irq;
> +
> + return xen_irq_info_common_setup(info, irq, IRQT_IPI, evtchn, 0);
> }
>
> -static void xen_irq_info_virq_init(unsigned cpu,
> +static int xen_irq_info_virq_setup(unsigned cpu,
> unsigned irq,
> unsigned short evtchn,
> unsigned short virq)
> {
> struct irq_info *info = info_for_irq(irq);
>
> - xen_irq_info_common_init(info, irq, IRQT_VIRQ, evtchn, 0);
> -
> info->u.virq = virq;
>
> per_cpu(virq_to_irq, cpu)[virq] = irq;
> +
> + return xen_irq_info_common_setup(info, irq, IRQT_VIRQ, evtchn, 0);
> }
>
> -static void xen_irq_info_pirq_init(unsigned irq,
> +static int xen_irq_info_pirq_setup(unsigned irq,
> unsigned short evtchn,
> unsigned short pirq,
> unsigned short gsi,
> @@ -163,12 +165,12 @@ static void xen_irq_info_pirq_init(unsigned irq,
> {
> struct irq_info *info = info_for_irq(irq);
>
> - xen_irq_info_common_init(info, irq, IRQT_PIRQ, evtchn, 0);
> -
> info->u.pirq.pirq = pirq;
> info->u.pirq.gsi = gsi;
> info->u.pirq.domid = domid;
> info->u.pirq.flags = flags;
> +
> + return xen_irq_info_common_setup(info, irq, IRQT_PIRQ, evtchn, 0);
> }
>
> /*
> @@ -516,6 +518,47 @@ int xen_irq_from_gsi(unsigned gsi)
> }
> EXPORT_SYMBOL_GPL(xen_irq_from_gsi);
>
> +static void __unbind_from_irq(unsigned int irq)
> +{
> + struct evtchn_close close;
> + int evtchn = evtchn_from_irq(irq);
> + struct irq_info *info = irq_get_handler_data(irq);
> +
> + if (info->refcnt > 0) {
> + info->refcnt--;
> + if (info->refcnt != 0)
> + return;
> + }
> +
> + if (VALID_EVTCHN(evtchn)) {
> + close.port = evtchn;
> + if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> + BUG();
> +
> + switch (type_from_irq(irq)) {
> + case IRQT_VIRQ:
> + per_cpu(virq_to_irq, cpu_from_evtchn(evtchn))
> + [virq_from_irq(irq)] = -1;
> + break;
> + case IRQT_IPI:
> + per_cpu(ipi_to_irq, cpu_from_evtchn(evtchn))
> + [ipi_from_irq(irq)] = -1;
> + break;
> + default:
> + break;
> + }
> +
> + /* Closed ports are implicitly re-bound to VCPU0. */
> + bind_evtchn_to_cpu(evtchn, 0);
> +
> + evtchn_to_irq[evtchn] = -1;
> + }
> +
> + BUG_ON(info_for_irq(irq)->type == IRQT_UNBOUND);
> +
> + xen_free_irq(irq);
> +}
> +
> /*
> * Do not make any assumptions regarding the relationship between the
> * IRQ number returned here and the Xen pirq argument.
> @@ -531,6 +574,7 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
> {
> int irq = -1;
> struct physdev_irq irq_op;
> + int ret;
>
> mutex_lock(&irq_mapping_update_lock);
>
> @@ -558,8 +602,13 @@ int xen_bind_pirq_gsi_to_irq(unsigned gsi,
> goto out;
> }
>
> - xen_irq_info_pirq_init(irq, 0, pirq, gsi, DOMID_SELF,
> + ret = xen_irq_info_pirq_setup(irq, 0, pirq, gsi, DOMID_SELF,
> shareable ? PIRQ_SHAREABLE : 0);
> + if (ret < 0) {
> + __unbind_from_irq(irq);
> + irq = ret;
> + goto out;
> + }
>
> pirq_query_unmask(irq);
> /* We try to use the handler with the appropriate semantic for the
> @@ -619,7 +668,9 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
> irq_set_chip_and_handler_name(irq, &xen_pirq_chip, handle_edge_irq,
> name);
>
> - xen_irq_info_pirq_init(irq, 0, pirq, 0, domid, 0);
> + ret = xen_irq_info_pirq_setup(irq, 0, pirq, 0, domid, 0);
> + if (ret < 0)
> + goto error_irq;
> ret = irq_set_msi_desc(irq, msidesc);
> if (ret < 0)
> goto error_irq;
> @@ -627,8 +678,8 @@ out:
> mutex_unlock(&irq_mapping_update_lock);
> return irq;
> error_irq:
> + __unbind_from_irq(irq);
> mutex_unlock(&irq_mapping_update_lock);
> - xen_free_irq(irq);
> return ret;
> }
> #endif
> @@ -698,9 +749,11 @@ int xen_pirq_from_irq(unsigned irq)
> return pirq_from_irq(irq);
> }
> EXPORT_SYMBOL_GPL(xen_pirq_from_irq);
> +
> int bind_evtchn_to_irq(unsigned int evtchn)
> {
> int irq;
> + int ret;
>
> mutex_lock(&irq_mapping_update_lock);
>
> @@ -714,7 +767,12 @@ int bind_evtchn_to_irq(unsigned int evtchn)
> irq_set_chip_and_handler_name(irq, &xen_dynamic_chip,
> handle_edge_irq, "event");
>
> - xen_irq_info_evtchn_init(irq, evtchn);
> + ret = xen_irq_info_evtchn_setup(irq, evtchn);
> + if (ret < 0) {
> + __unbind_from_irq(irq);
> + irq = ret;
> + goto out;
> + }
> } else {
> struct irq_info *info = info_for_irq(irq);
> WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
> @@ -731,6 +789,7 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
> {
> struct evtchn_bind_ipi bind_ipi;
> int evtchn, irq;
> + int ret;
>
> mutex_lock(&irq_mapping_update_lock);
>
> @@ -750,8 +809,12 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
> BUG();
> evtchn = bind_ipi.port;
>
> - xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
> -
> + ret = xen_irq_info_ipi_setup(cpu, irq, evtchn, ipi);
> + if (ret < 0) {
> + __unbind_from_irq(irq);
> + irq = ret;
> + goto out;
> + }
> bind_evtchn_to_cpu(evtchn, cpu);
> } else {
> struct irq_info *info = info_for_irq(irq);
> @@ -830,7 +893,12 @@ int bind_virq_to_irq(unsigned int virq, unsigned int cpu)
> evtchn = ret;
> }
>
> - xen_irq_info_virq_init(cpu, irq, evtchn, virq);
> + ret = xen_irq_info_virq_setup(cpu, irq, evtchn, virq);
> + if (ret < 0) {
> + __unbind_from_irq(irq);
> + irq = ret;
> + goto out;
> + }
>
> bind_evtchn_to_cpu(evtchn, cpu);
> } else {
> @@ -846,50 +914,8 @@ out:
>
> static void unbind_from_irq(unsigned int irq)
> {
> - struct evtchn_close close;
> - int evtchn = evtchn_from_irq(irq);
> - struct irq_info *info = irq_get_handler_data(irq);
> -
> - if (WARN_ON(!info))
> - return;
> -
> mutex_lock(&irq_mapping_update_lock);
> -
> - if (info->refcnt > 0) {
> - info->refcnt--;
> - if (info->refcnt != 0)
> - goto done;
> - }
> -
> - if (VALID_EVTCHN(evtchn)) {
> - close.port = evtchn;
> - if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> - BUG();
> -
> - switch (type_from_irq(irq)) {
> - case IRQT_VIRQ:
> - per_cpu(virq_to_irq, cpu_from_evtchn(evtchn))
> - [virq_from_irq(irq)] = -1;
> - break;
> - case IRQT_IPI:
> - per_cpu(ipi_to_irq, cpu_from_evtchn(evtchn))
> - [ipi_from_irq(irq)] = -1;
> - break;
> - default:
> - break;
> - }
> -
> - /* Closed ports are implicitly re-bound to VCPU0. */
> - bind_evtchn_to_cpu(evtchn, 0);
> -
> - evtchn_to_irq[evtchn] = -1;
> - }
> -
> - BUG_ON(info_for_irq(irq)->type == IRQT_UNBOUND);
> -
> - xen_free_irq(irq);
> -
> - done:
> + __unbind_from_irq(irq);
> mutex_unlock(&irq_mapping_update_lock);
> }
>
> @@ -1137,7 +1163,7 @@ void rebind_evtchn_irq(int evtchn, int irq)
> so there should be a proper type */
> BUG_ON(info->type == IRQT_UNBOUND);
>
> - xen_irq_info_evtchn_init(irq, evtchn);
> + xen_irq_info_evtchn_setup(irq, evtchn);
>
> mutex_unlock(&irq_mapping_update_lock);
>
> @@ -1312,7 +1338,7 @@ static void restore_cpu_virqs(unsigned int cpu)
> evtchn = bind_virq.port;
>
> /* Record the new mapping. */
> - xen_irq_info_virq_init(cpu, irq, evtchn, virq);
> + xen_irq_info_virq_setup(cpu, irq, evtchn, virq);
> bind_evtchn_to_cpu(evtchn, cpu);
> }
> }
> @@ -1336,7 +1362,7 @@ static void restore_cpu_ipis(unsigned int cpu)
> evtchn = bind_ipi.port;
>
> /* Record the new mapping. */
> - xen_irq_info_ipi_init(cpu, irq, evtchn, ipi);
> + xen_irq_info_ipi_setup(cpu, irq, evtchn, ipi);
> bind_evtchn_to_cpu(evtchn, cpu);
> }
> }
> --
> 1.7.2.5
>
next prev parent reply other threads:[~2013-09-24 14:47 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-13 16:59 (no subject) David Vrabel
2013-09-13 16:59 ` [PATCH 01/13] xen/events: refactor retrigger_dynirq() and resend_irq_on_evtchn() David Vrabel
2013-09-24 14:37 ` Konrad Rzeszutek Wilk
2013-09-24 16:04 ` David Vrabel
2013-09-13 16:59 ` [PATCH 02/13] xen/events: remove unnecessary init_evtchn_cpu_bindings() David Vrabel
2013-09-24 14:40 ` Konrad Rzeszutek Wilk
2013-09-24 16:06 ` David Vrabel
2013-09-24 16:49 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 03/13] xen/events: introduce test_and_set_mask David Vrabel
2013-09-24 14:40 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 04/13] xen/events: replace raw bit ops with functions David Vrabel
2013-09-24 14:41 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 05/13] xen/events: move drivers/xen/events.c into drivers/xen/events/ David Vrabel
2013-09-13 16:59 ` [PATCH 06/13] xen/events: move 2-level specific code into its own file David Vrabel
2013-09-13 16:59 ` [PATCH 07/13] xen/events: add struct evtchn_ops for the low-level port operations David Vrabel
2013-09-24 14:44 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 08/13] xen/events: allow setup of irq_info to fail David Vrabel
2013-09-24 14:47 ` Konrad Rzeszutek Wilk [this message]
2013-09-13 16:59 ` [PATCH 09/13] xen/events: add a evtchn_op for port setup David Vrabel
2013-09-24 14:47 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 10/13] xen/events: Refactor evtchn_to_irq array to be dynamically allocated David Vrabel
2013-09-24 14:58 ` Konrad Rzeszutek Wilk
2013-09-26 10:53 ` David Vrabel
2013-09-26 12:51 ` Konrad Rzeszutek Wilk
2013-09-13 16:59 ` [PATCH 11/13] xen/events: add xen_evtchn_mask_all() David Vrabel
2013-09-24 14:58 ` Konrad Rzeszutek Wilk
2013-09-13 17:00 ` [PATCH 12/13] xen/events: Add the hypervisor interface for the FIFO-based event channels David Vrabel
2013-09-24 15:06 ` Konrad Rzeszutek Wilk
2013-09-26 11:06 ` David Vrabel
2013-09-24 15:08 ` Konrad Rzeszutek Wilk
2013-09-24 16:11 ` David Vrabel
2013-09-24 16:51 ` Konrad Rzeszutek Wilk
2013-09-13 17:00 ` [PATCH 13/13] xen/events: use the FIFO-based ABI if available David Vrabel
2013-09-24 15:50 ` Konrad Rzeszutek Wilk
2013-09-24 16:48 ` David Vrabel
2013-09-24 17:04 ` Konrad Rzeszutek Wilk
2013-09-13 17:03 ` [RFC PATCHv3 00/12] Linux: FIFO-based event channel ABI David Vrabel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130924144703.GG4712@phenom.dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=boris.ostrovsky@oracle.com \
--cc=david.vrabel@citrix.com \
--cc=jbeulich@suse.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).