thr3ads.net - Xen devel - [PATCH 0/2] xen-evtchn misc fixes [Feb 2013]

If this information is useful, please help other people find it:
Share via:

Wei Liu

2013-Feb-18 14:57 UTC

[PATCH 0/2] xen-evtchn misc fixes

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.

Wei Liu

2013-Feb-18 14:57 UTC

head link

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

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

Wei Liu

2013-Feb-18 14:57 UTC

head link

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

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

Konrad Rzeszutek Wilk

2013-Feb-19 14:32 UTC

head link

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

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
>

Wei Liu

2013-Feb-19 14:46 UTC

head link

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

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

Wei Liu

2013-Feb-19 14:50 UTC

head link

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

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

Konrad Rzeszutek Wilk

2013-Feb-19 14:54 UTC

head link

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

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

Xen devel - Feb 2013 - [PATCH 0/2] xen-evtchn misc fixes

[PATCH 0/2] xen-evtchn misc fixes

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

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

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

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

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

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