Re: [PATCH 2/6] bsd-user/freebsd/os-syscall.c: unlock_iovec

[Richard Henderson]

On 6/7/22 16:35, Warner Losh wrote:
> > On Jun 7, 2022, at 3:23 PM, Richard Henderson <richard.henderson@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~