lkml.org 
[lkml]   [2019]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 3/3] vfio_pci: make use of update_irq_devid and optimize irq ops
On Thu, 15 Aug 2019, Ben Luo wrote:
> if (vdev->ctx[vector].trigger) {
> - free_irq(irq, vdev->ctx[vector].trigger);
> - irq_bypass_unregister_producer(&vdev->ctx[vector].producer);
> - kfree(vdev->ctx[vector].name);
> - eventfd_ctx_put(vdev->ctx[vector].trigger);
> - vdev->ctx[vector].trigger = NULL;
> + if (vdev->ctx[vector].trigger == trigger) {
> + irq_bypass_unregister_producer(&vdev->ctx[vector].producer);

What's the point of unregistering the producer, just to register it again below?

> + } else if (trigger) {
> + ret = update_irq_devid(irq,
> + vdev->ctx[vector].trigger, trigger);
> + if (unlikely(ret)) {
> + dev_info(&pdev->dev,
> + "update_irq_devid %d (token %p) fails: %d\n",

s/fails/failed/

> + irq, vdev->ctx[vector].trigger, ret);
> + eventfd_ctx_put(trigger);
> + return ret;
> + }

This lacks any form of comment why this is correct. dev_id is updated and
the producer with the old token is still registered.

> + irq_bypass_unregister_producer(&vdev->ctx[vector].producer);

Now it's unregistered.

> + eventfd_ctx_put(vdev->ctx[vector].trigger);
> + vdev->ctx[vector].producer.token = trigger;

The token is updated and then it's newly registered below.

> + vdev->ctx[vector].trigger = trigger;
> - vdev->ctx[vector].producer.token = trigger;
> - vdev->ctx[vector].producer.irq = irq;
> + /* always update irte for posted mode */
> ret = irq_bypass_register_producer(&vdev->ctx[vector].producer);
> if (unlikely(ret))
> dev_info(&pdev->dev,
> "irq bypass producer (token %p) registration fails: %d\n",
> vdev->ctx[vector].producer.token, ret);

I know this code already existed, but again this looks inconsistent. If the
registration fails then a subsequent update will try to unregister a not
registered producer. Does not make any sense to me.

Thanks,

tglx

\
 
 \ /
  Last update: 2019-08-15 18:46    [W:0.118 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site