> +static int bsg_transport_complete_rq(struct request *rq, struct sg_io_v4 
> *hdr)
> +{
> +     struct bsg_job *job = blk_mq_rq_to_pdu(rq);
> +     int ret = 0;
> +
> +     /*
> +      * The assignments below don't make much sense, but are kept for
> +      * bug by bug backwards compatibility:
> +      */
> +     hdr->device_status = job->result & 0xff;
> +     hdr->transport_status = host_byte(job->result);
> +     hdr->driver_status = driver_byte(job->result);
> +     hdr->info = 0;
> +     if (hdr->device_status || hdr->transport_status || hdr->driver_status)
> +             hdr->info |= SG_INFO_CHECK;
> +     hdr->response_len = 0;
> +
> +     if (job->result < 0) {
> +             /* we're only returning the result field in the reply */
> +             job->reply_len = sizeof(u32);
> +             ret = job->result;
> +     }
> +
> +     if (job->reply_len && hdr->response) {
> +             int len = min(hdr->max_response_len, job->reply_len);
> +
> +             if (copy_to_user(ptr64(hdr->response), job->reply, len))
> +                     ret = -EFAULT;
> +             else
> +                     hdr->response_len = len;
> +     }
> +
> +     /* we assume all request payload was transferred, residual == 0 */
> +     hdr->dout_resid = 0;
> +
> +     if (rq->next_rq) {
> +             unsigned int rsp_len = blk_rq_bytes(rq->next_rq);
> +
> +             if (WARN_ON(job->reply_payload_rcv_len > rsp_len))

This gives my a lot of new Warnings when running my tests on zFCP, non
of which happen when I run on 4.13.

After browsing the source some, I figured this is because in comparison
to the old code, blk_rq_bytes() is now called after we finished the
blk-request already and blk_update_bidi_request()+blk_update_request()
was already called. This will update rq->__data_len, and thus the call
here to get rsp_len will get a wrong value. Thus the warnings, and the
following calculation is actually wrong.

I figure you can just replace this call for blk_rq_bytes() with
'job->reply_payload.payload_len'. Its essentially the same value, but
the later is not changed after the job is committed as far as I could see
in the source. Driver makes use of it, but only reading as far as I
could make out after browsing the code for a bit.

I did a quick test with that change in place and that seems to work fine
now. As far as my tests go, they behave as they did before.


                                                    Beste Grüße / Best regards,
                                                      - Benjamin Block

> +                     hdr->din_resid = 0;
> +             else
> +                     hdr->din_resid = rsp_len - job->reply_payload_rcv_len;
> +     } else {
> +             hdr->din_resid = 0;
> +     }
> +
> +     return ret;
> +}
> +

--
Linux on z Systems Development         /         IBM Systems & Technology Group
                  IBM Deutschland Research & Development GmbH
Vorsitz. AufsR.: Martina Koederitz     /        Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: AmtsG Stuttgart, HRB 243294

Reply via email to