Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1488430 > unrolled thread

[rfc] weirdness in bio_map_user_iov()

Back to article view | Back to linux.kernel

Contents

  [rfc] weirdness in bio_map_user_iov() Al Viro <viro@ZenIV.linux.org.uk> - 2016-09-22 00:00 +0200
    Re: [rfc] weirdness in bio_map_user_iov() Sagi Grimberg <sagi@grimberg.me> - 2016-09-23 23:50 +0200

#1488430 — [rfc] weirdness in bio_map_user_iov()

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?

[toc] | [next] | [standalone]

#1490477

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().

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web