On Thu, Jan 05, 2023 at 09:48:10AM +0800, Pengfei Xu wrote:
> Hi Viro,
>
> It's a soft remind: "_copy_from_iter" WARNING issue was still reproduced
> in v6.2-rc2 mainline kernel in guest.
sorry, had been sick ;-/
I see what's going on there; it's this bit:
if ((iov_iter_rw(iter) == WRITE &&
(!map_data || !map_data->null_mapped)) ||
(map_data && map_data->from_user)) {
ret = bio_copy_from_iter(bio, iter);
if (ret)
goto cleanup;
The "map_data && map_data->from_user" part is what I'd missed.
Mea culpa.
That code really is reading from destination here; in a lot of
respects that's broken (consider the case when destination is
mapped write-only), but it's a deliberate behaviour.
Origin is in commit ecb554a846f8 "block: fix sg SG_DXFER_TO_FROM_DEV
regression". Before trying to kludge around that, how much of the
original problem is still there?
Are there still low-level drivers that don't bother with scsi_set_resid()
for passthrough requests? IOW, do we need to play with
copy the entire destination into kernel buffer
handle REQ_OP_DRV_IN request, overwriting the copy
copy the entire buffer back to destination
all because we can't tell how much data had been copied in?
Because if we had the length of that sucker in sg_rq_end_io(),
we could just arrange for bio->bmd->iter truncated to actual
amount read and that would be it.
NOTE: no other users of blk_rq_map_user_io() go anywhere near that
weirdness; it's really just /dev/sg*.
If we that's approach is not feasible, we can always deal with
that in bio_copy_from_iter() - forcibly flip ->data_source for the
duration, which is OK for all kinds of iov_iter that can reach that
thing; it would work, but that's really ugly ;-/ If there's really
no way to get the amount of data actually transferred, that's what
we'll have to do, but it would be much saner to avoid the entire
song and dance and just copy the right amount out...
