Commit c3bfffa5 authored Jul 03, 2020 by Julian Wiedmann Committed by Martin K. Petersen Jul 08, 2020

scsi: zfcp: Avoid benign overflow of the Request Queue's free-level

zfcp_qdio_send() and zfcp_qdio_int_req() run concurrently, adding and
completing SBALs on the Request Queue. There's a theoretical race where
zfcp_qdio_int_req() completes a number of SBALs & increments the queue's
free-level _before_ zfcp_qdio_send() was able to decrement it.

This can cause ->req_q_free to momentarily hold a value larger than
QDIO_MAX_BUFFERS_PER_Q. Luckily zfcp_qdio_send() is always called under
->req_q_lock, and all readers of the free-level also take this lock. So we
can trust that zfcp_qdio_send() will clean up such a temporary overflow
before anyone can actually observe it.

But it's still confusing and annoying to worry about. So adjust the code to
avoid this race.

Link: https://lore.kernel.org/r/7f61f59a1f8db270312e64644f9173b8f1ac895f.1593780621.git.bblock@linux.ibm.com


Reviewed-by: Steffen Maier <maier@linux.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

parent 6bcb7c17

drivers/s390/scsi/zfcp_qdio.c

+4 −1

Original line number	Diff line number	Diff line
		@@ -260,17 +260,20 @@ int zfcp_qdio_send(struct zfcp_qdio qdio, struct zfcp_qdio_req q_req)
		zfcp_qdio_account(qdio);
		spin_unlock(&qdio->stat_lock);

		atomic_sub(sbal_number, &qdio->req_q_free);

		retval = do_QDIO(qdio->adapter->ccw_device, QDIO_FLAG_SYNC_OUTPUT, 0,
		q_req->sbal_first, sbal_number);

		if (unlikely(retval)) {
		/* Failed to submit the IO, roll back our modifications. */
		atomic_add(sbal_number, &qdio->req_q_free);
		zfcp_qdio_zero_sbals(qdio->req_q, q_req->sbal_first,
		sbal_number);
		return retval;
		}

		/* account for transferred buffers */
		atomic_sub(sbal_number, &qdio->req_q_free);
		qdio->req_q_idx += sbal_number;
		qdio->req_q_idx %= QDIO_MAX_BUFFERS_PER_Q;

Admin message