Commit 058b8647 authored by Swen Schillig's avatar Swen Schillig Committed by James Bottomley
Browse files

[SCSI] zfcp: Replace fsf_req wait_queue with completion 



The combination wait_queue/wakeup in conjunction with the flag
ZFCP_STATUS_FSFREQ_COMPLETED to signal the completion of an fsfreq
was not race-safe and can be better solved by a completion.

Signed-off-by: default avatarSwen Schillig <swen@vnet.ibm.com>
Signed-off-by: default avatarChristof Schmitt <christof.schmitt@de.ibm.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@suse.de>
parent bd63eaf4

drivers/s390/scsi/zfcp_def.h

+1 −2
Original line number Diff line number Diff line
@@ -248,7 +248,6 @@ enum zfcp_wka_status { 


/* FSF request status (this does not have a common part) */

#define ZFCP_STATUS_FSFREQ_TASK_MANAGEMENT	0x00000002

#define ZFCP_STATUS_FSFREQ_COMPLETED		0x00000004

#define ZFCP_STATUS_FSFREQ_ERROR		0x00000008

#define ZFCP_STATUS_FSFREQ_CLEANUP		0x00000010

#define ZFCP_STATUS_FSFREQ_ABORTSUCCEEDED	0x00000040
@@ -532,7 +531,7 @@ struct zfcp_fsf_req { 
	u8		       sbale_curr;     /* current SBALE during creation

						  of request */

	u8			sbal_response;	/* SBAL used in interrupt */

	wait_queue_head_t      completion_wq;  /* can be used by a routine

	struct completion	completion;	/* can be used by a routine

						  to wait for completion */

	u32			status;	       /* status of this request */

	u32		       fsf_command;    /* FSF Command copy */

drivers/s390/scsi/zfcp_erp.c

+1 −2
Original line number Diff line number Diff line
@@ -485,8 +485,7 @@ static void zfcp_erp_strategy_check_fsfreq(struct zfcp_erp_action *act) 
		}

		if (act->status & ZFCP_STATUS_ERP_TIMEDOUT)

			zfcp_rec_dbf_event_action("erscf_2", act);

		if (act->fsf_req->status & (ZFCP_STATUS_FSFREQ_COMPLETED |

					    ZFCP_STATUS_FSFREQ_DISMISSED))

		if (act->fsf_req->status & ZFCP_STATUS_FSFREQ_DISMISSED)

			act->fsf_req = NULL;

	} else

		act->fsf_req = NULL;

drivers/s390/scsi/zfcp_fsf.c

+6 −20
Original line number Diff line number Diff line
@@ -444,23 +444,11 @@ static void zfcp_fsf_req_complete(struct zfcp_fsf_req *req) 


	if (req->erp_action)

		zfcp_erp_notify(req->erp_action, 0);

	req->status |= ZFCP_STATUS_FSFREQ_COMPLETED;



	if (likely(req->status & ZFCP_STATUS_FSFREQ_CLEANUP))

		zfcp_fsf_req_free(req);

	else

	/* notify initiator waiting for the requests completion */

	/*

	 * FIXME: Race! We must not access fsf_req here as it might have been

	 * cleaned up already due to the set ZFCP_STATUS_FSFREQ_COMPLETED

	 * flag. It's an improbable case. But, we have the same paranoia for

	 * the cleanup flag already.

	 * Might better be handled using complete()?

	 * (setting the flag and doing wakeup ought to be atomic

	 *  with regard to checking the flag as long as waitqueue is

	 *  part of the to be released structure)

	 */

		wake_up(&req->completion_wq);

		complete(&req->completion);

}



/**
@@ -733,7 +721,7 @@ static struct zfcp_fsf_req *zfcp_fsf_req_create(struct zfcp_adapter *adapter, 


	INIT_LIST_HEAD(&req->list);

	init_timer(&req->timer);

	init_waitqueue_head(&req->completion_wq);

	init_completion(&req->completion);



	req->adapter = adapter;

	req->fsf_command = fsf_cmd;
@@ -1309,8 +1297,7 @@ int zfcp_fsf_exchange_config_data_sync(struct zfcp_adapter *adapter, 
	retval = zfcp_fsf_req_send(req);

	spin_unlock_bh(&adapter->req_q_lock);

	if (!retval)

		wait_event(req->completion_wq,

			   req->status & ZFCP_STATUS_FSFREQ_COMPLETED);

		wait_for_completion(&req->completion);



	zfcp_fsf_req_free(req);

	return retval;
@@ -1405,8 +1392,8 @@ int zfcp_fsf_exchange_port_data_sync(struct zfcp_adapter *adapter, 
	spin_unlock_bh(&adapter->req_q_lock);



	if (!retval)

		wait_event(req->completion_wq,

			   req->status & ZFCP_STATUS_FSFREQ_COMPLETED);

		wait_for_completion(&req->completion);



	zfcp_fsf_req_free(req);



	return retval;
@@ -2572,8 +2559,7 @@ struct zfcp_fsf_req *zfcp_fsf_control_file(struct zfcp_adapter *adapter, 
	spin_unlock_bh(&adapter->req_q_lock);



	if (!retval) {

		wait_event(req->completion_wq,

			   req->status & ZFCP_STATUS_FSFREQ_COMPLETED);

		wait_for_completion(&req->completion);

		return req;

	}

	return ERR_PTR(retval);

drivers/s390/scsi/zfcp_scsi.c

+2 −4
Original line number Diff line number Diff line
@@ -206,8 +206,7 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt) 
	if (!abrt_req)

		return FAILED;



	wait_event(abrt_req->completion_wq,

		   abrt_req->status & ZFCP_STATUS_FSFREQ_COMPLETED);

	wait_for_completion(&abrt_req->completion);



	if (abrt_req->status & ZFCP_STATUS_FSFREQ_ABORTSUCCEEDED)

		dbf_tag = "okay";
@@ -246,8 +245,7 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags) 
	if (!fsf_req)

		return FAILED;



	wait_event(fsf_req->completion_wq,

		   fsf_req->status & ZFCP_STATUS_FSFREQ_COMPLETED);

	wait_for_completion(&fsf_req->completion);



	if (fsf_req->status & ZFCP_STATUS_FSFREQ_TMFUNCFAILED) {

		zfcp_scsi_dbf_event_devreset("fail", tm_flags, unit, scpnt);