| From: Julian Wiedmann <jwi@linux.vnet.ibm.com> |
| Date: Wed, 7 Mar 2018 14:01:01 +0100 |
| Subject: s390/qdio: don't merge ERROR output buffers |
| |
| commit 0cf1e05157b9e5530dcc3ca9fec9bf617fc93375 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: 104ea556ee7f ("qdio: support asynchronous delivery of storage blocks") |
| Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com> |
| Reviewed-by: Ursula Braun <ubraun@linux.vnet.ibm.com> |
| Reviewed-by: Benjamin Block <bblock@linux.vnet.ibm.com> |
| Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| drivers/s390/cio/qdio_main.c | 31 ++++++++++++++++++++----------- |
| 1 file changed, 20 insertions(+), 11 deletions(-) |
| |
| --- a/drivers/s390/cio/qdio_main.c |
| +++ b/drivers/s390/cio/qdio_main.c |
| @@ -212,7 +212,10 @@ again: |
| 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, |
| unsigned char *state, unsigned int count, |
| int auto_ack, int merge_pending) |
| @@ -223,17 +226,23 @@ static inline int get_buf_states(struct |
| if (is_qebsm(q)) |
| return qdio_do_eqbs(q, state, bufnr, count, auto_ack); |
| |
| - for (i = 0; i < count; i++) { |
| - if (!__state) { |
| - __state = q->slsb.val[bufnr]; |
| - if (merge_pending && __state == SLSB_P_OUTPUT_PENDING) |
| - __state = SLSB_P_OUTPUT_EMPTY; |
| - } else if (merge_pending) { |
| - if ((q->slsb.val[bufnr] & __state) != __state) |
| - break; |
| - } else if (q->slsb.val[bufnr] != __state) |
| - break; |
| + /* get initial state: */ |
| + __state = q->slsb.val[bufnr]; |
| + if (merge_pending && __state == SLSB_P_OUTPUT_PENDING) |
| + __state = SLSB_P_OUTPUT_EMPTY; |
| + |
| + for (i = 1; i < count; i++) { |
| 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; |
| return i; |