Re: [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.

[John Garry <john.garry@xxxxxxxxxx>]

> +static irqreturn_t be_iopoll(struct be_eq_obj *pbe_eq)
> +{
> +	struct beiscsi_hba *phba;
> +	unsigned int ret, io_events;
> +	struct be_eq_entry *eqe = NULL;
> +	struct be_queue_info *eq;
> +
> +	phba = pbe_eq->phba;
> +	if (beiscsi_hba_in_error(phba))
> +		return IRQ_NONE;
> +
> +	io_events = 0;
> +	eq = &pbe_eq->q;
> +	eqe = queue_tail_node(eq);
> +	while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32] &
> +			EQE_VALID_MASK) {
> +		AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
> +		queue_tail_inc(eq);
> +		eqe = queue_tail_node(eq);
> +		io_events++;
> +	}
> +	hwi_ring_eq_db(phba, eq->id, 1, io_events, 0, 1);
> +
> +	ret = beiscsi_process_cq(pbe_eq);
> +	pbe_eq->cq_count += ret;
> +	beiscsi_log(phba, KERN_INFO,
> +		    BEISCSI_LOG_CONFIG | BEISCSI_LOG_IO,
> +		    "BM_%d : rearm pbe_eq->q.id =%d ret %d\n",
> +		    pbe_eq->q.id, ret);
> +	if (!beiscsi_hba_in_error(phba))
> +		hwi_ring_eq_db(phba, pbe_eq->q.id, 0, 0, 1, 1);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t be_isr_misx_th(int irq, void *dev_id)
> +{
> +	struct be_eq_obj *pbe_eq  = dev_id;
> +
> +	return be_iopoll(pbe_eq);
> +}
> +
>   /**
>    * be_isr_msix - The isr routine of the driver.
>    * @irq: Not used
> @@ -713,9 +752,22 @@ static irqreturn_t be_isr_msix(int irq, void *dev_id)
>   	phba = pbe_eq->phba;
>   	/* disable interrupt till iopoll completes */
>   	hwi_ring_eq_db(phba, eq->id, 1,	0, 0, 1);
> -	irq_poll_sched(&pbe_eq->iopoll);
>   
> -	return IRQ_HANDLED;
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t be_isr_thread(int irq, void *dev_id)
> +{
> +	struct beiscsi_hba *phba;
> +	struct hwi_controller *phwi_ctrlr;
> +	struct hwi_context_memory *phwi_context;
> +	struct be_eq_obj *pbe_eq;
> +
> +	phba = dev_id;
> +	phwi_ctrlr = phba->phwi_ctrlr;
> +	phwi_context = phwi_ctrlr->phwi_ctxt;
> +	pbe_eq = &phwi_context->be_eq[0];
> +	return be_iopoll(pbe_eq);
>   }
>   
>   /**
> @@ -735,6 +787,7 @@ static irqreturn_t be_isr(int irq, void *dev_id)
>   	struct be_ctrl_info *ctrl;
>   	struct be_eq_obj *pbe_eq;
>   	int isr, rearm;
> +	irqreturn_t ret;
>   
>   	phba = dev_id;
>   	ctrl = &phba->ctrl;
> @@ -774,10 +827,11 @@ static irqreturn_t be_isr(int irq, void *dev_id)
>   		/* rearm for MCCQ */
>   		rearm = 1;
>   	}
> +	ret = IRQ_HANDLED;
>   	if (io_events)
> -		irq_poll_sched(&pbe_eq->iopoll);
> +		ret = IRQ_WAKE_THREAD;
>   	hwi_ring_eq_db(phba, eq->id, 0, (io_events + mcc_events), rearm, 1);
> -	return IRQ_HANDLED;
> +	return ret;
>   }
>   
>   static void beiscsi_free_irqs(struct beiscsi_hba *phba)
> @@ -819,9 +873,10 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
>   				goto free_msix_irqs;
>   			}
>   
> -			ret = request_irq(pci_irq_vector(pcidev, i),
> -					  be_isr_msix, 0, phba->msi_name[i],
> -					  &phwi_context->be_eq[i]);
> +			ret = request_threaded_irq(pci_irq_vector(pcidev, i),
> +						   be_isr_msix, be_isr_misx_th,
> +						   0, phba->msi_name[i],
> +						   &phwi_context->be_eq[i]);

Would it be sensible to set ONESHOT flag here? I assume that we don't 
want the hard irq handler firing continuously while we poll completions 
in the threaded handler. That's my understanding of how ONESHOT should 
or would work... they currently seem to manually disable in 
be_isr_msix() -> hwi_ring_eq_db() for the same purpose, I guess.

Thanks,
John