Messages in this thread | | | Date | Tue, 24 Apr 2018 16:20:50 +0200 | From | Takashi Iwai <> | Subject | Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling |
| |
On Mon, 16 Apr 2018 08:24:51 +0200, Oleksandr Andrushchenko wrote: > +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id) > +{ > + struct xen_snd_front_evtchnl *channel = dev_id; > + struct xen_snd_front_info *front_info = channel->front_info; > + struct xensnd_resp *resp; > + RING_IDX i, rp; > + unsigned long flags; > + > + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED)) > + return IRQ_HANDLED; > + > + spin_lock_irqsave(&front_info->io_lock, flags); > + > +again: > + rp = channel->u.req.ring.sring->rsp_prod; > + /* ensure we see queued responses up to rp */ > + rmb(); > + > + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a very long loop, no? Better to have a sanity check of the ring buffer size.
> +static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id) > +{ > + struct xen_snd_front_evtchnl *channel = dev_id; > + struct xen_snd_front_info *front_info = channel->front_info; > + struct xensnd_event_page *page = channel->u.evt.page; > + u32 cons, prod; > + unsigned long flags; > + > + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED)) > + return IRQ_HANDLED; > + > + spin_lock_irqsave(&front_info->io_lock, flags); > + > + prod = page->in_prod; > + /* ensure we see ring contents up to prod */ > + virt_rmb(); > + if (prod == page->in_cons) > + goto out; > + > + for (cons = page->in_cons; cons != prod; cons++) {
Ditto.
thanks,
Takashi
| |