xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
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
> 

  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).