Commit 12d4eebb authored by Julian Wiedmann's avatar Julian Wiedmann Committed by Stefan Bader

s390/qdio: don't merge ERROR output buffers

BugLink: http://bugs.launchpad.net/bugs/1768429

commit 0cf1e051 upstream.

On an Output queue, both EMPTY and PENDING buffer states imply that the
buffer is ready for completion-processing by the upper-layer drivers.

So for a non-QEBSM Output queue, get_buf_states() merges mixed
batches of PENDING and EMPTY buffers into one large batch of EMPTY
buffers. The upper-layer driver (ie. qeth) later distuingishes PENDING
from EMPTY by inspecting the slsb_state for
QDIO_OUTBUF_STATE_FLAG_PENDING.

But the merge logic in get_buf_states() contains a bug that causes us to
erronously also merge ERROR buffers into such a batch of EMPTY buffers
(ERROR is 0xaf, EMPTY is 0xa1; so ERROR & EMPTY == EMPTY).
Effectively, most outbound ERROR buffers are currently discarded
silently and processed as if they had succeeded.

Note that this affects _all_ non-QEBSM device types, not just IQD with CQ.

Fix it by explicitly spelling out the exact conditions for merging.

For extracting the "get initial state" part out of the loop, this relies
on the fact that get_buf_states() is never called with a count of 0. The
QEBSM path already strictly requires this, and the two callers with
variable 'count' make sure of it.

Fixes: 104ea556 ("qdio: support asynchronous delivery of storage blocks")
Cc: <stable@vger.kernel.org> #v3.2+
Signed-off-by: default avatarJulian Wiedmann <jwi@linux.vnet.ibm.com>
Reviewed-by: default avatarUrsula Braun <ubraun@linux.vnet.ibm.com>
Reviewed-by: default avatarBenjamin Block <bblock@linux.vnet.ibm.com>
Signed-off-by: default avatarMartin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent e26f4135
...@@ -205,7 +205,10 @@ static int qdio_do_sqbs(struct qdio_q *q, unsigned char state, int start, ...@@ -205,7 +205,10 @@ static int qdio_do_sqbs(struct qdio_q *q, unsigned char state, int start,
return 0; return 0;
} }
/* returns number of examined buffers and their common state in *state */ /*
* Returns number of examined buffers and their common state in *state.
* Requested number of buffers-to-examine must be > 0.
*/
static inline int get_buf_states(struct qdio_q *q, unsigned int bufnr, static inline int get_buf_states(struct qdio_q *q, unsigned int bufnr,
unsigned char *state, unsigned int count, unsigned char *state, unsigned int count,
int auto_ack, int merge_pending) int auto_ack, int merge_pending)
...@@ -216,17 +219,23 @@ static inline int get_buf_states(struct qdio_q *q, unsigned int bufnr, ...@@ -216,17 +219,23 @@ static inline int get_buf_states(struct qdio_q *q, unsigned int bufnr,
if (is_qebsm(q)) if (is_qebsm(q))
return qdio_do_eqbs(q, state, bufnr, count, auto_ack); return qdio_do_eqbs(q, state, bufnr, count, auto_ack);
for (i = 0; i < count; i++) { /* get initial state: */
if (!__state) { __state = q->slsb.val[bufnr];
__state = q->slsb.val[bufnr]; if (merge_pending && __state == SLSB_P_OUTPUT_PENDING)
if (merge_pending && __state == SLSB_P_OUTPUT_PENDING) __state = SLSB_P_OUTPUT_EMPTY;
__state = SLSB_P_OUTPUT_EMPTY;
} else if (merge_pending) { for (i = 1; i < count; i++) {
if ((q->slsb.val[bufnr] & __state) != __state)
break;
} else if (q->slsb.val[bufnr] != __state)
break;
bufnr = next_buf(bufnr); bufnr = next_buf(bufnr);
/* merge PENDING into EMPTY: */
if (merge_pending &&
q->slsb.val[bufnr] == SLSB_P_OUTPUT_PENDING &&
__state == SLSB_P_OUTPUT_EMPTY)
continue;
/* stop if next state differs from initial state: */
if (q->slsb.val[bufnr] != __state)
break;
} }
*state = __state; *state = __state;
return i; return i;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment