Re: [rfc] weirdness in bio_map_user_iov()

2016-09-23 Thread Sagi Grimberg

Hey Al,


What happens if we feed it a 3-element iovec array, one page in each?
AFAICS, bio_add_pc_page() is called for each of those pages, even if
the previous calls have failed - break is only out of the inner loop.

Sure, failure due to exceeded request size means that everything after
that one will fail, but what of e.g.
/*
 * If the queue doesn't support SG gaps and adding this
 * offset would create a gap, disallow it.
 */
if (bvec_gap_to_prev(q, prev, offset))
return 0;
in there?  Won't we risk having the first and the third pages added, with
the second one quietly skipped?  Jens, looks like it had come from you
(by way of jejb).  Am I missing something subtle here?


So AFAICT, 'gappy' iovecs will never reach bio_map_user_iov() because it
is checked before in blk_rq_map_user_iov():

if (map_data)
copy = true;
else if (iov_iter_alignment(iter) & align)
copy = true;
else if (queue_virt_boundary(q))
copy = queue_virt_boundary(q) & 
iov_iter_gap_alignment(iter);


if iov_iter_gap_alignment(iter) detects gaps it will use
a nice aligned bounce via bio_copy_user_iov().
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[rfc] weirdness in bio_map_user_iov()

2016-09-21 Thread Al Viro
What happens if we feed it a 3-element iovec array, one page in each?
AFAICS, bio_add_pc_page() is called for each of those pages, even if
the previous calls have failed - break is only out of the inner loop.

Sure, failure due to exceeded request size means that everything after
that one will fail, but what of e.g.
/*
 * If the queue doesn't support SG gaps and adding this
 * offset would create a gap, disallow it.
 */
if (bvec_gap_to_prev(q, prev, offset))
return 0;
in there?  Won't we risk having the first and the third pages added, with
the second one quietly skipped?  Jens, looks like it had come from you
(by way of jejb).  Am I missing something subtle here?
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html