Re: [RFC][PATCH 2/6] fnic: add fnic_scsi.c and fnic_io.h.

[Mike Christie <michaelc@cs.wisc.edu>]

jeykholt@cisco.com wrote:
> fnic: add fnic_scsi.c and fnic_io.h.
> 
> fnic_scsi.c contains the FCP SCSI handling as well as firmware reset
> and FLOGI registration handling.
> 

I just looked at this one function, because I was fixing the same code 
in fcoe.ko/libfc:fc_fcp.c


> +int fnic_reset(struct Scsi_Host *shost)
> +{
> +	struct fc_lport *lp;
> +	struct fnic *fnic;
> +	unsigned long flags;
> +	int ret = SUCCESS;
> +	enum fnic_state old_state;
> +	DECLARE_COMPLETION_ONSTACK(reset_wait);
> +
> +	lp = shost_priv(shost);
> +	fnic = lp->drv_priv;
> +
> +	printk(KERN_DEBUG DFX "fnic_reset called\n", fnic->fnic_no);
> +
> +	/* Issue firmware reset */
> +	spin_lock_irqsave(&fnic->fnic_lock, flags);
> +	fnic->reset_wait = &reset_wait;
> +	old_state = fnic->state;
> +	fnic->state = FNIC_IN_FC_TRANS_ETH_MODE;
> +	vnic_dev_del_addr(fnic->vdev, fnic->data_src_addr);
> +	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +
> +	if (fnic_fw_reset_handler(fnic)) {
> +		spin_lock_irqsave(&fnic->fnic_lock, flags);
> +		ret = FAILED;
> +		if (fnic->state == FNIC_IN_FC_TRANS_ETH_MODE)
> +			fnic->state = old_state;
> +		fnic->reset_wait = NULL;
> +		spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +		goto fnic_reset_end;
> +	}
> +
> +	/* fw reset is issued, now wait for it to complete */
> +	wait_for_completion_timeout(&reset_wait,
> +				    msecs_to_jiffies(FNIC_HOST_RESET_TIMEOUT));
> +
> +	/* Check for status */
> +	spin_lock_irqsave(&fnic->fnic_lock, flags);
> +	fnic->reset_wait = NULL;
> +	ret = (fnic->state == FNIC_IN_ETH_MODE) ? SUCCESS : FAILED;
> +	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +
> +	/* Now reset local port, this will clean up libFC exchanges,
> +	 * reset remote port sessions, and if link is up, begin flogi
> +	 */
> +	fc_lport_lock(lp);
> +	if (lp->tt.lport_reset(lp))
> +		ret = FAILED;


The problem here is that this only starts the login. When fnic_reset 
returns for the scsi eh path, scsi-ml is going to send a TUR to make 
sure that we are ready to go. If we are not the devices will be 
offlined. So unless it is a really quick relogin we are going to offline 
the devices by accident.

For fc_fcp.c I added a hokey loop and wait like some other drivers. We 
could instead have libfc notify any waiters of a state change here. We 
could also do a rport blocked timedout helper, convert the fc drivers 
and use it here so we only wait for the login to complete or for the 
port block to fail.




> +	fc_lport_unlock(lp);
> +
> +fnic_reset_end:
> +	printk(KERN_DEBUG DFX "Returning from fnic reset %s\n",
> +	       fnic->fnic_no, (ret == SUCCESS) ? "SUCCESS" : "FAILED");
> +
> +	return ret;
> +}
> +
> +/* SCSI Error handling calls driver's eh_host_reset if all prior
> + * error handling levels return FAILED. If host reset completes
> + * successfully, and if link is up, then Fabric login begins.
> + *
> + * Host Reset is the highest level of error recovery. If this fails, then
> + * host is offlined by SCSI.
> + *
> + */
> +int fnic_host_reset(struct scsi_cmnd *sc)
> +{
> +	return fnic_reset(sc->device->host);
> +}
> +
> +/*
> + * This fxn is called from libFC when host is removed
> + */
> +void fnic_scsi_abort_io(struct fc_lport *lp)
> +{
> +	int err = 0;
> +	unsigned long flags;
> +	enum fnic_state old_state;
> +	struct fnic *fnic = lp->drv_priv;
> +	DECLARE_COMPLETION_ONSTACK(remove_wait);
> +
> +	/* Issue firmware reset for fnic, wait for reset to complete */
> +	spin_lock_irqsave(&fnic->fnic_lock, flags);
> +	fnic->remove_wait = &remove_wait;
> +	old_state = fnic->state;
> +	fnic->state = FNIC_IN_FC_TRANS_ETH_MODE;
> +	vnic_dev_del_addr(fnic->vdev, fnic->data_src_addr);
> +	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +
> +	err = fnic_fw_reset_handler(fnic);
> +	if (err) {
> +		spin_lock_irqsave(&fnic->fnic_lock, flags);
> +		if (fnic->state == FNIC_IN_FC_TRANS_ETH_MODE)
> +			fnic->state = old_state;
> +		fnic->remove_wait = NULL;
> +		spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +		goto abort_io_end;
> +	}
> +
> +	/* Wait for firmware reset to complete */
> +	wait_for_completion_timeout(&remove_wait,
> +				    msecs_to_jiffies(FNIC_RMDEVICE_TIMEOUT));
> +
> +	spin_lock_irqsave(&fnic->fnic_lock, flags);
> +	fnic->remove_wait = NULL;
> +	printk(KERN_DEBUG DFX "fnic_scsi_abort_io %s\n", fnic->fnic_no,
> +	       (fnic->state == FNIC_IN_ETH_MODE) ? "SUCCESS" : "FAILED");
> +	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +
> +abort_io_end:
> +	return;
> +}
> +
> +/*
> + * This fxn called from libFC to clean up driver IO state on link down
> + */
> +void fnic_scsi_cleanup(struct fc_lport *lp)
> +{
> +	unsigned long flags;
> +	enum fnic_state old_state;
> +	struct fnic *fnic = lp->drv_priv;
> +
> +	/* issue fw reset */
> +	spin_lock_irqsave(&fnic->fnic_lock, flags);
> +	old_state = fnic->state;
> +	fnic->state = FNIC_IN_FC_TRANS_ETH_MODE;
> +	vnic_dev_del_addr(fnic->vdev, fnic->data_src_addr);
> +	spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +
> +	if (fnic_fw_reset_handler(fnic)) {
> +		spin_lock_irqsave(&fnic->fnic_lock, flags);
> +		if (fnic->state == FNIC_IN_FC_TRANS_ETH_MODE)
> +			fnic->state = old_state;
> +		spin_unlock_irqrestore(&fnic->fnic_lock, flags);
> +	}
> +
> +}
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html