Re: [PATCH] 9p/xen: fix release of IRQ

[Jürgen Groß <jgross@xxxxxxxx>]

On 22.11.24 00:44, Dominique Martinet wrote:
> Alexander Merritt wrote on Thu, Nov 21, 2024 at 10:51:00PM +0000:
> > From: Alex Zenla <alex@xxxxxxxxx>
> >
> > Kernel logs indicate an IRQ was double-freed.
> Nit: if you still have the log it'd be great to include it in the commit
> message, rather than paragraphing it.
>
> The rationale is that someone with the same problem will likely just
> search for the error as is first, and having it in the commit log will
> be an easy hit.
>
> (This alone wouldn't need a resend, I can add it if you just reply to
> the mail with it; it's also fine if you no longer have the log, that'll
> be a remark for the next patch)
>
>
> > Pass correct device ID during IRQ release.
> >
> > Fixes: 71ebd71921e45 ("xen/9pfs: connect to the backend")
> > Signed-off-by: Alex Zenla <alex@xxxxxxxxx>
> > Signed-off-by: Alexander Merritt <alexander@xxxxxxxxx>
> > Signed-off-by: Ariadne Conill <ariadne@ariadne.space>
>
>
> > ---
> >   net/9p/trans_xen.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> > index dfdbe1ca5338..198d46d79d84 100644
> > --- a/net/9p/trans_xen.c
> > +++ b/net/9p/trans_xen.c
> > @@ -286,7 +286,8 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv 
> > *priv)
> >                 if (!priv->rings[i].intf)
> >                         break;
> >                 if (priv->rings[i].irq > 0)
> > -                       unbind_from_irqhandler(priv->rings[i].irq, priv->dev);
> > +                       unbind_from_irqhandler(priv->rings[i].irq, ring);
> > +               priv->rings[i].evtchn = priv->rings[i].irq = 0;
> (style) I don't recall seeing much `a = b = 0` in the kernel, and
> looking at it checkpatch seems to complain:
> CHECK: multiple assignments should be avoided
> #114: FILE: net/9p/trans_xen.c:290:
> +               priv->rings[i].evtchn = priv->rings[i].irq = 0;
>
> Please run checkpatch on the patches you send (b4 can do it for you if
> you want to start using it)
>
>
> code-wise,
> I also don't see where unbinf_from_irqhandler would free the evtchn, so
> is it leaking here, or is it implicit from something else?
> We only free it explicitly on error binding the irq.
unbind_from_irqhandler()
  unbind_from_irq()
    __unbind_from_irq()
      close_evtchn()


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature