On Fri, Sep 23, 2022 at 05:38:19PM +0200, Christoph Hellwig wrote: > >> + else { >> + struct iovec fast_iov[UIO_FASTIOV]; >> + struct iovec *iov = fast_iov; >> + struct iov_iter iter; >> + >> + ret = import_iovec(rq_data_dir(req), ubuffer, bufflen, >> + UIO_FASTIOV, &iov, &iter); >> + if (ret < 0) >> goto out; >> + ret = blk_rq_map_user_iov(q, req, NULL, &iter, GFP_KERNEL); >> + kfree(iov); >> + } > >While you touch this: I think thi block of code would also be a good >separate helper. Maybe even in the block layer given the the scsi >ioctl code and sg duplicate it, and already missed the fast_iov >treatment due to the duplication. Having this in a separate function >is also nice to keep the fast_iov stack footprint isolated. Totally agree on goodness. I think instead of new helper this seems suited to go inside blk_rq_map_user_iov itself. That will make it symmetric to blk_rq_map_user which also combines import + mapping. But if I go that route now, I will have to alter parameters of blk_rq_map_user_iov, and that will make it mandatory to change the callers (scsi-ioctl, sg) too. Nothing hairy, but that means further growth of unrelated elements in this series. Hope you agree that separate series is much better, which I will post after this. Will fold all other changes you pointed.