[PATCH 0/2] xen-evtchn misc fixes

[PATCH 0/2] xen-evtchn misc fixes

[Wei Liu]

Two misc fixes for Xen evtchn driver. The first one is a reposted version of a
bug fix, the second one is comment and output adjustment.

[PATCH 1/2] xen: close evtchn port if binding to irq fails

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/xen/evtchn.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index b1f60a0..b2db77e 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -269,6 +269,14 @@ static int evtchn_bind_to_user(struct per_user_data *u, int port)
 				       u->name, (void *)(unsigned long)port);
 	if (rc >= 0)
 		rc = evtchn_make_refcounted(port);
+	else {
+		/* bind failed, should close the port now */
+		struct evtchn_close close;
+		close.port = port;
+		if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
+			BUG();
+		set_port_user(port, NULL);
+	}
 
 	return rc;
 }
@@ -277,6 +285,8 @@ static void evtchn_unbind_from_user(struct per_user_data *u, int port)
 {
 	int irq = irq_from_evtchn(port);
 
+	BUG_ON(irq < 0);
+
 	unbind_from_irqhandler(irq, (void *)(unsigned long)port);
 
 	set_port_user(port, NULL);
-- 
1.7.10.4

[PATCH 2/2] xen-evtchn: correct comment and error output

[Wei Liu]

The evtchn device has been moved to /dev/xen. Also change log level to
KERN_ERR as other xen drivers.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/xen/evtchn.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index b2db77e..45c8efa 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -544,10 +544,10 @@ static int __init evtchn_init(void)
 
 	spin_lock_init(&port_user_lock);
 
-	/* Create '/dev/misc/evtchn'. */
+	/* Create '/dev/xen/evtchn'. */
 	err = misc_register(&evtchn_miscdev);
 	if (err != 0) {
-		printk(KERN_ALERT "Could not register /dev/misc/evtchn\n");
+		printk(KERN_ERR "Could not register /dev/xen/evtchn\n");
 		return err;
 	}
 
-- 
1.7.10.4

Re: [PATCH 1/2] xen: close evtchn port if binding to irq fails

[Konrad Rzeszutek Wilk]

On Mon, Feb 18, 2013 at 02:57:58PM +0000, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

I thought we had discussed doing this check in the user of
this as well?

> ---
>  drivers/xen/evtchn.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index b1f60a0..b2db77e 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -269,6 +269,14 @@ static int evtchn_bind_to_user(struct per_user_data *u, int port)
>  				       u->name, (void *)(unsigned long)port);
>  	if (rc >= 0)
>  		rc = evtchn_make_refcounted(port);
> +	else {
> +		/* bind failed, should close the port now */
> +		struct evtchn_close close;
> +		close.port = port;
> +		if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> +			BUG();
> +		set_port_user(port, NULL);
> +	}
>  
>  	return rc;
>  }
> @@ -277,6 +285,8 @@ static void evtchn_unbind_from_user(struct per_user_data *u, int port)
>  {
>  	int irq = irq_from_evtchn(port);
>  
> +	BUG_ON(irq < 0);
> +
>  	unbind_from_irqhandler(irq, (void *)(unsigned long)port);
>  
>  	set_port_user(port, NULL);
> -- 
> 1.7.10.4
> 

Re: [PATCH 1/2] xen: close evtchn port if binding to irq fails

[Wei Liu]

On Tue, 2013-02-19 at 14:32 +0000, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 18, 2013 at 02:57:58PM +0000, Wei Liu wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> I thought we had discussed doing this check in the user of
> this as well?
> 

Hah? I remember Jan's comment on the port == 0 case and your suggestion
to add BUG_ON() in the unbind path. Did I miss anything?


Wei.

> > ---
> >  drivers/xen/evtchn.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> > index b1f60a0..b2db77e 100644
> > --- a/drivers/xen/evtchn.c
> > +++ b/drivers/xen/evtchn.c
> > @@ -269,6 +269,14 @@ static int evtchn_bind_to_user(struct per_user_data *u, int port)
> >  				       u->name, (void *)(unsigned long)port);
> >  	if (rc >= 0)
> >  		rc = evtchn_make_refcounted(port);
> > +	else {
> > +		/* bind failed, should close the port now */
> > +		struct evtchn_close close;
> > +		close.port = port;
> > +		if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> > +			BUG();
> > +		set_port_user(port, NULL);
> > +	}
> >  
> >  	return rc;
> >  }
> > @@ -277,6 +285,8 @@ static void evtchn_unbind_from_user(struct per_user_data *u, int port)
> >  {
> >  	int irq = irq_from_evtchn(port);
> >  
> > +	BUG_ON(irq < 0);
> > +
> >  	unbind_from_irqhandler(irq, (void *)(unsigned long)port);
> >  
> >  	set_port_user(port, NULL);
> > -- 
> > 1.7.10.4
> > 

Re: [PATCH 1/2] xen: close evtchn port if binding to irq fails

[Wei Liu]

On Tue, 2013-02-19 at 14:32 +0000, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 18, 2013 at 02:57:58PM +0000, Wei Liu wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> I thought we had discussed doing this check in the user of
> this as well?
> 

User of this? I presume you mean the binding path? The patch also does
this. The first patch issues close hypercall in the unbind path, but
this patch issues hypercall in the bind path, and adds BUG_ON in the
unbind path.

As for port == 0, this cannot happend, so I remove the port == 0 case.


Wei.

> > ---
> >  drivers/xen/evtchn.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> > index b1f60a0..b2db77e 100644
> > --- a/drivers/xen/evtchn.c
> > +++ b/drivers/xen/evtchn.c
> > @@ -269,6 +269,14 @@ static int evtchn_bind_to_user(struct per_user_data *u, int port)
> >  				       u->name, (void *)(unsigned long)port);
> >  	if (rc >= 0)
> >  		rc = evtchn_make_refcounted(port);
> > +	else {
> > +		/* bind failed, should close the port now */
> > +		struct evtchn_close close;
> > +		close.port = port;
> > +		if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> > +			BUG();
> > +		set_port_user(port, NULL);
> > +	}
> >  
> >  	return rc;
> >  }
> > @@ -277,6 +285,8 @@ static void evtchn_unbind_from_user(struct per_user_data *u, int port)
> >  {
> >  	int irq = irq_from_evtchn(port);
> >  
> > +	BUG_ON(irq < 0);
> > +
> >  	unbind_from_irqhandler(irq, (void *)(unsigned long)port);
> >  
> >  	set_port_user(port, NULL);
> > -- 
> > 1.7.10.4
> > 

Re: [PATCH 1/2] xen: close evtchn port if binding to irq fails

[Konrad Rzeszutek Wilk]

On Tue, Feb 19, 2013 at 02:46:31PM +0000, Wei Liu wrote:
> On Tue, 2013-02-19 at 14:32 +0000, Konrad Rzeszutek Wilk wrote:
> > On Mon, Feb 18, 2013 at 02:57:58PM +0000, Wei Liu wrote:
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > I thought we had discussed doing this check in the user of
> > this as well?
> > 
> 
> Hah? I remember Jan's comment on the port == 0 case and your suggestion
> to add BUG_ON() in the unbind path. Did I miss anything?

This patch by itself is fine. I thought that we had chatted of adding
a check in evtchn_bind_to_user to check the return value and so
something about it. But the more I think about this - it makes less
sense than doing it here.

So this patch is good - putting it on my 3.9 list.
> 
> 
> Wei.
> 
> > > ---
> > >  drivers/xen/evtchn.c |   10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> > > index b1f60a0..b2db77e 100644
> > > --- a/drivers/xen/evtchn.c
> > > +++ b/drivers/xen/evtchn.c
> > > @@ -269,6 +269,14 @@ static int evtchn_bind_to_user(struct per_user_data *u, int port)
> > >  				       u->name, (void *)(unsigned long)port);
> > >  	if (rc >= 0)
> > >  		rc = evtchn_make_refcounted(port);
> > > +	else {
> > > +		/* bind failed, should close the port now */
> > > +		struct evtchn_close close;
> > > +		close.port = port;
> > > +		if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> > > +			BUG();
> > > +		set_port_user(port, NULL);
> > > +	}
> > >  
> > >  	return rc;
> > >  }
> > > @@ -277,6 +285,8 @@ static void evtchn_unbind_from_user(struct per_user_data *u, int port)
> > >  {
> > >  	int irq = irq_from_evtchn(port);
> > >  
> > > +	BUG_ON(irq < 0);
> > > +
> > >  	unbind_from_irqhandler(irq, (void *)(unsigned long)port);
> > >  
> > >  	set_port_user(port, NULL);
> > > -- 
> > > 1.7.10.4
> > > 
> 
>