[PATCH] Make bind_interdomain_evtchn_to_irq() public

[PATCH] Make bind_interdomain_evtchn_to_irq() public

[jgross]

From: Juergen Gross <jgross@suse.com>

bind_interdomain_evtchn_to_irq() is currently a private function. It is used
only by bind_interdomain_evtchn_to_irqhandler() to register an irq-handler for
an event channel.

By making it public it is possible to use e.g. threaded interrupts with
interdomain event channels.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/events/events_base.c | 5 +++--
 include/xen/events.h             | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c919d3d..0230aff 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -900,8 +900,8 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
 	return irq;
 }
 
-static int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
-					  unsigned int remote_port)
+int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
+				   unsigned int remote_port)
 {
 	struct evtchn_bind_interdomain bind_interdomain;
 	int err;
@@ -914,6 +914,7 @@ static int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
 
 	return err ? : bind_evtchn_to_irq(bind_interdomain.local_port);
 }
+EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq);
 
 static int find_virq(unsigned int virq, unsigned int cpu)
 {
diff --git a/include/xen/events.h b/include/xen/events.h
index 8bee7a7..5321cd9 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -28,6 +28,8 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
 			   unsigned long irqflags,
 			   const char *devname,
 			   void *dev_id);
+int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
+				   unsigned int remote_port);
 int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
 					  unsigned int remote_port,
 					  irq_handler_t handler,
-- 
1.8.4.5

Re: [Xen-devel] [PATCH] Make bind_interdomain_evtchn_to_irq() public

[David Vrabel]

On 12/08/14 14:43, jgross@suse.com wrote:
> From: Juergen Gross <jgross@suse.com>
> 
> bind_interdomain_evtchn_to_irq() is currently a private function. It is used
> only by bind_interdomain_evtchn_to_irqhandler() to register an irq-handler for
> an event channel.
> 
> By making it public it is possible to use e.g. threaded interrupts with
> interdomain event channels.

Can you give an example of using this?  Maybe we want a
bind_interdomain_evtchn_to_threaded_irqhandler() instead? Or add a flags
field to the existing function.

David

> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -900,8 +900,8 @@ static int bind_ipi_to_irq(unsigned int ipi, unsigned int cpu)
>  	return irq;
>  }
>  
> -static int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
> -					  unsigned int remote_port)
> +int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
> +				   unsigned int remote_port)
>  {
>  	struct evtchn_bind_interdomain bind_interdomain;
>  	int err;
> @@ -914,6 +914,7 @@ static int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
>  
>  	return err ? : bind_evtchn_to_irq(bind_interdomain.local_port);
>  }
> +EXPORT_SYMBOL_GPL(bind_interdomain_evtchn_to_irq);
>  
>  static int find_virq(unsigned int virq, unsigned int cpu)
>  {
> diff --git a/include/xen/events.h b/include/xen/events.h
> index 8bee7a7..5321cd9 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -28,6 +28,8 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
>  			   unsigned long irqflags,
>  			   const char *devname,
>  			   void *dev_id);
> +int bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
> +				   unsigned int remote_port);
>  int bind_interdomain_evtchn_to_irqhandler(unsigned int remote_domain,
>  					  unsigned int remote_port,
>  					  irq_handler_t handler,
> 

Re: [Xen-devel] [PATCH] Make bind_interdomain_evtchn_to_irq() public

[Juergen Gross]

On 08/13/2014 11:29 AM, David Vrabel wrote:
> On 12/08/14 14:43, jgross@suse.com wrote:
>> From: Juergen Gross <jgross@suse.com>
>>
>> bind_interdomain_evtchn_to_irq() is currently a private function. It is used
>> only by bind_interdomain_evtchn_to_irqhandler() to register an irq-handler for
>> an event channel.
>>
>> By making it public it is possible to use e.g. threaded interrupts with
>> interdomain event channels.
>
> Can you give an example of using this?  Maybe we want a
> bind_interdomain_evtchn_to_threaded_irqhandler() instead? Or add a flags
> field to the existing function.

Instead of

static int scsiback_schedule(void *data)
{
        struct vscsibk_info *info = (struct vscsibk_info *)data;

        while (!kthread_should_stop()) {
                wait_event_interruptible(info->wq,
                        info->waiting_reqs || kthread_should_stop());

                info->waiting_reqs = 0;
                smp_mb();       /* waiting_reqs used by other thread */

                switch (scsiback_do_cmd_fn(info)) {
                case 1:
                        info->waiting_reqs = 1;
                default:
                        break;
                }
                cond_resched();
        }
        return 0;
}

static void scsiback_notify_work(struct vscsibk_info *info)
{
        info->waiting_reqs = 1;
        wake_up(&info->wq);
}

static irqreturn_t scsiback_intr(int irq, void *dev_id)
{
        scsiback_notify_work((struct vscsibk_info *)dev_id);
        return IRQ_HANDLED;
}

static int scsiback_init_sring(struct vscsibk_info *info, grant_ref_t 
ring_ref,
                        evtchn_port_t evtchn)
{
...
        bind_interdomain_evtchn_to_irqhandler(
                        info->domid, evtchn,
                        scsiback_intr, 0, "vscsiif-backend", info);

...
        info->kthread = kthread_run(scsiback_schedule, info, name);
...
}


I only need the following:

static irqreturn_t scsiback_irq_fn(int irq, void *dev_id)
{
         struct vscsibk_info *info = dev_id;

         while (scsiback_do_cmd_fn(info))
                 cond_resched();

         return IRQ_HANDLED;
}

static int scsiback_init_sring(struct vscsibk_info *info, grant_ref_t 
ring_ref,
                         evtchn_port_t evtchn)
{
...
         irq = bind_interdomain_evtchn_to_irq(info->domid, evtchn);
         request_threaded_irq(irq, NULL, scsiback_irq_fn,
                              IRQF_ONESHOT, "vscsiif-backend", info)
...
}


So yes, it could be done via a flag, too.

Juergen

Re: [PATCH] Make bind_interdomain_evtchn_to_irq() public

[David Vrabel]

On 12/08/14 14:43, jgross@suse.com wrote:
> From: Juergen Gross <jgross@suse.com>
> 
> bind_interdomain_evtchn_to_irq() is currently a private function. It is used
> only by bind_interdomain_evtchn_to_irqhandler() to register an irq-handler for
> an event channel.
> 
> By making it public it is possible to use e.g. threaded interrupts with
> interdomain event channels.

Acked-by: David Vrabel <david.vrabel@citrix.com>

But can you change the subject/commit message to:

    xen/events: support threaded irqs for interdomain event channels

    Export bind_interdomain_evtchn_to_irq() so drivers can use threaded
    interrupt handlers with:

     irq = bind_interdomain_evtchn_to_irq(remote_dom, remote_port);
     if (irq < 0)
         /* error */
     ret = request_threaded_irq(...);

Since your pvscsi series depends on this you will want to add this to
that series to be merged via the scsi tree.

David

Re: [Xen-devel] [PATCH] Make bind_interdomain_evtchn_to_irq() public

[David Vrabel]

On 13/08/14 10:43, Juergen Gross wrote:
> 
> static int scsiback_init_sring(struct vscsibk_info *info, grant_ref_t
> ring_ref,
>                         evtchn_port_t evtchn)
> {
> ...
>         irq = bind_interdomain_evtchn_to_irq(info->domid, evtchn);
>         request_threaded_irq(irq, NULL, scsiback_irq_fn,
>                              IRQF_ONESHOT, "vscsiif-backend", info)
> ...

This usage is fine.

I assume the error checking of bind_interdomain_evtchn_to_irq() and
request_threaded_irq() was omitted for clarity in this example?

David

Re: [Xen-devel] [PATCH] Make bind_interdomain_evtchn_to_irq() public

[Juergen Gross]

On 08/13/2014 11:53 AM, David Vrabel wrote:
> On 12/08/14 14:43, jgross@suse.com wrote:
>> From: Juergen Gross <jgross@suse.com>
>>
>> bind_interdomain_evtchn_to_irq() is currently a private function. It is used
>> only by bind_interdomain_evtchn_to_irqhandler() to register an irq-handler for
>> an event channel.
>>
>> By making it public it is possible to use e.g. threaded interrupts with
>> interdomain event channels.
>
> Acked-by: David Vrabel <david.vrabel@citrix.com>
>
> But can you change the subject/commit message to:
>
>      xen/events: support threaded irqs for interdomain event channels
>
>      Export bind_interdomain_evtchn_to_irq() so drivers can use threaded
>      interrupt handlers with:
>
>       irq = bind_interdomain_evtchn_to_irq(remote_dom, remote_port);
>       if (irq < 0)
>           /* error */
>       ret = request_threaded_irq(...);
>
> Since your pvscsi series depends on this you will want to add this to
> that series to be merged via the scsi tree.

Already done locally. :-)


Juergen

Re: [Xen-devel] [PATCH] Make bind_interdomain_evtchn_to_irq() public

[Juergen Gross]

On 08/13/2014 11:54 AM, David Vrabel wrote:
> On 13/08/14 10:43, Juergen Gross wrote:
>>
>> static int scsiback_init_sring(struct vscsibk_info *info, grant_ref_t
>> ring_ref,
>>                          evtchn_port_t evtchn)
>> {
>> ...
>>          irq = bind_interdomain_evtchn_to_irq(info->domid, evtchn);
>>          request_threaded_irq(irq, NULL, scsiback_irq_fn,
>>                               IRQF_ONESHOT, "vscsiif-backend", info)
>> ...
>
> This usage is fine.
>
> I assume the error checking of bind_interdomain_evtchn_to_irq() and
> request_threaded_irq() was omitted for clarity in this example?

Sure.


Juergen