On Sun, Jun 6, 2021 at 12:08 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> It really needs to be cleaned up, without performance loss -
> this stuff *does* sit on performance-critical paths. We also need to
> fix a bunch of corner cases in there.
>
> The series attempting to do so {...}
Hmm. Each individual patch looks sensible to me. Not that I tested
them - just from reading through them. So I might well have missed
something.
The end result certainly looks cleaner than before, although I can't
say that it's pretty. It's still haitry and grotty, but at least
_some_ of the hairiness has been removed (yay for
_copy_from_iter_full() being *much* simpler now, and
iterate_all_kinds() being gone).
I also have to admit to despising what iov_iter_init() ends up looking
like. Not because I think that using those initializer assignments to
set it is wrong, but simply because the initializers are basically
identical except for "iter_type".
Yeah, yeah, there is also the kvec/iov" difference, but those fields
are actually a union, and you're assigning the same value to them in
both cases, and the way you do it now requires a cast for the kvec
case.
So I think iov_iter_init() would actually be better off just being
*i = (struct iov_iter) {
.iter_type = uaccess_kernel() ? ITER_KVEC : ITER_IOVEC,
.data_source = direction,
.iov = iov,
.nr_segs = nr_segs,
.iov_offset = 0,
.count = count
};
with possibly a big comment about that ".opv = iov" thing being a
union member assignment, and being either a iovec or a kvec.
That makes the code simpler, and avoids the cast that is otherwise necessary.
Hmm?
Linus
