On 6/7/22 16:35, Warner Losh wrote:


On Jun 7, 2022, at 3:23 PM, Richard Henderson <richard.hender...@linaro.org> 
wrote:

On 6/7/22 14:51, Warner Losh wrote:
    void unlock_iovec(IOVecMap *map, bool copy_out)
    {
          for (int i = 0, count = map->count; i < count; ++i) {
              if (map->host[i].iov_base) {
                  abi_ulong target_base = tswapal(map->target[i].iov_base);
                  unlock_user(map->host[i].iov_base, target_base,
                              copy_out ? map->host[i].iov_len : 0);
              }
And wouldn't we want to filter out the iov_base that == 0 since
we may terminate the loop before we get to the count. When the
I/O is done, we'll call it not with the number we mapped, but with
the original number...  Or am I not understanding something here...

I'm not following -- when and why are you adjusting count?

When we hit a memory range we can’t map after the first one,
we effectively stop mapping in (in the current linux code we
do map after, but then destroy the length). So that means
we’ll have entries in the iovec that are zero, and this code
doesn’t account for that. We’re not changing the count, per
se, but have a scenario where they might wind up NULL.

... and so skip them with the if.

I mean, I suppose you could set map->count on error, as you say, so that we don't iterate so far, but... duh, error case. So long as you don't actively fail, there's no point in optimizing for it.


r~

Reply via email to