Re: [PATCH 4/9] drm/xen-front: Implement Xen event channel handling

On 02/21/2018 03:03 AM, Oleksandr Andrushchenko wrote:

+static irqreturn_t evtchnl_interrupt_ctrl(int irq, void *dev_id) +{

• struct xen_drm_front_evtchnl *evtchnl = dev_id;
• struct xen_drm_front_info *front_info = evtchnl->front_info;
• struct xendispl_resp *resp;
• RING_IDX i, rp;
• unsigned long flags;
• spin_lock_irqsave(&front_info->io_lock, flags);
• if (unlikely(evtchnl->state != EVTCHNL_STATE_CONNECTED))

• goto out;
  

Do you need to check the state under lock? (in other routines too).

...

+static void evtchnl_free(struct xen_drm_front_info *front_info,

• struct xen_drm_front_evtchnl *evtchnl)
  

+{

• unsigned long page = 0;
• if (evtchnl->type == EVTCHNL_TYPE_REQ)

• page = (unsigned long)evtchnl->u.req.ring.sring;
  

• else if (evtchnl->type == EVTCHNL_TYPE_EVT)

• page = (unsigned long)evtchnl->u.evt.page;
  

• if (!page)

• return;
  

• evtchnl->state = EVTCHNL_STATE_DISCONNECTED;
• if (evtchnl->type == EVTCHNL_TYPE_REQ) {

• /* release all who still waits for response if any */
  

• evtchnl->u.req.resp_status = -EIO;
  

• complete_all(&evtchnl->u.req.completion);
  

• }
• if (evtchnl->irq)

• unbind_from_irqhandler(evtchnl->irq, evtchnl);
  

• if (evtchnl->port)

• xenbus_free_evtchn(front_info->xb_dev, evtchnl->port);
  

• /* end access and free the page */
• if (evtchnl->gref != GRANT_INVALID_REF)

• gnttab_end_foreign_access(evtchnl->gref, 0, page);
  

• if (evtchnl->type == EVTCHNL_TYPE_REQ)

• evtchnl->u.req.ring.sring = NULL;
  

• else

• evtchnl->u.evt.page = NULL;
  

• memset(evtchnl, 0, sizeof(*evtchnl));

Since you are zeroing out the structure you don't need to set fields to zero.

I also think you need to free the page.

-boris