Gu Zheng <[email protected]> writes:
> use an iovec array rather than the single one, so that we can avoid
> to alloc more iovecs buffer in small(< 8) PREADV/PWRITEV cases.
I did some basic functional testing of this change and the change in
patch 1/4. That testing included using aio-stress to drive queue depths
of 7, 8 and 9, and verify that it didn't fall over. I also ran xfstests
'./check -g aio', and libaio's 'make partcheck'.
The change looks good to me, and passed testing, so:
Reviewed-by: Jeff Moyer <[email protected]>
However, I still would like some comment on the reasoning behind it, and
whether there is some measurable performance advantage for some
workload. Additionally, it would be nice if that comment made its way
into the commit message.
Cheers,
Jeff
>
> Signed-off-by: Gu Zheng <[email protected]>
> ---
> fs/aio.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 0cd0479..ef21efe 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1260,12 +1260,12 @@ static ssize_t aio_setup_vectored_rw(struct kiocb *kiocb,
> if (compat)
> ret = compat_rw_copy_check_uvector(rw,
> (struct compat_iovec __user *)buf,
> - *nr_segs, 1, *iovec, iovec);
> + *nr_segs, UIO_FASTIOV, *iovec, iovec);
> else
> #endif
> ret = rw_copy_check_uvector(rw,
> (struct iovec __user *)buf,
> - *nr_segs, 1, *iovec, iovec);
> + *nr_segs, UIO_FASTIOV, *iovec, iovec);
> if (ret < 0)
> return ret;
>
> @@ -1302,7 +1302,7 @@ static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode,
> fmode_t mode;
> aio_rw_op *rw_op;
> rw_iter_op *iter_op;
> - struct iovec inline_vec, *iovec = &inline_vec;
> + struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
> struct iov_iter iter;
>
> switch (opcode) {
> @@ -1337,7 +1337,7 @@ rw_common:
> if (!ret)
> ret = rw_verify_area(rw, file, &req->ki_pos, req->ki_nbytes);
> if (ret < 0) {
> - if (iovec != &inline_vec)
> + if (iovec != inline_vecs)
> kfree(iovec);
> return ret;
> }
> @@ -1384,7 +1384,7 @@ rw_common:
> return -EINVAL;
> }
>
> - if (iovec != &inline_vec)
> + if (iovec != inline_vecs)
> kfree(iovec);
>
> if (ret != -EIOCBQUEUED) {
Hi Jeff,
On 07/22/2014 11:20 PM, Jeff Moyer wrote:
> Gu Zheng <[email protected]> writes:
>
>> use an iovec array rather than the single one, so that we can avoid
>> to alloc more iovecs buffer in small(< 8) PREADV/PWRITEV cases.
>
> I did some basic functional testing of this change and the change in
> patch 1/4. That testing included using aio-stress to drive queue depths
> of 7, 8 and 9, and verify that it didn't fall over. I also ran xfstests
> './check -g aio', and libaio's 'make partcheck'.
>
> The change looks good to me, and passed testing, so:
>
> Reviewed-by: Jeff Moyer <[email protected]>
Thanks for your review and test.
>
> However, I still would like some comment on the reasoning behind it, and
> whether there is some measurable performance advantage for some
> workload. Additionally, it would be nice if that comment made its way
> into the commit message.
I'll add more useful info, and send it out later.
Thanks,
Gu
>
> Cheers,
> Jeff
>
>>
>> Signed-off-by: Gu Zheng <[email protected]>
>> ---
>> fs/aio.c | 10 +++++-----
>> 1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index 0cd0479..ef21efe 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1260,12 +1260,12 @@ static ssize_t aio_setup_vectored_rw(struct kiocb *kiocb,
>> if (compat)
>> ret = compat_rw_copy_check_uvector(rw,
>> (struct compat_iovec __user *)buf,
>> - *nr_segs, 1, *iovec, iovec);
>> + *nr_segs, UIO_FASTIOV, *iovec, iovec);
>> else
>> #endif
>> ret = rw_copy_check_uvector(rw,
>> (struct iovec __user *)buf,
>> - *nr_segs, 1, *iovec, iovec);
>> + *nr_segs, UIO_FASTIOV, *iovec, iovec);
>> if (ret < 0)
>> return ret;
>>
>> @@ -1302,7 +1302,7 @@ static ssize_t aio_run_iocb(struct kiocb *req, unsigned opcode,
>> fmode_t mode;
>> aio_rw_op *rw_op;
>> rw_iter_op *iter_op;
>> - struct iovec inline_vec, *iovec = &inline_vec;
>> + struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
>> struct iov_iter iter;
>>
>> switch (opcode) {
>> @@ -1337,7 +1337,7 @@ rw_common:
>> if (!ret)
>> ret = rw_verify_area(rw, file, &req->ki_pos, req->ki_nbytes);
>> if (ret < 0) {
>> - if (iovec != &inline_vec)
>> + if (iovec != inline_vecs)
>> kfree(iovec);
>> return ret;
>> }
>> @@ -1384,7 +1384,7 @@ rw_common:
>> return -EINVAL;
>> }
>>
>> - if (iovec != &inline_vec)
>> + if (iovec != inline_vecs)
>> kfree(iovec);
>>
>> if (ret != -EIOCBQUEUED) {
> .
>