lkml.org 
[lkml]   [2018]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
Date


On 5/18/18 8:14 AM, Jens Axboe wrote:
> On 5/17/18 2:38 PM, adam.manzanares@wdc.com wrote:
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> This is the per-I/O equivalent of the ioprio_set system call.
>>
>> When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
>> newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
>>
>> When a bio is created for an aio request by the block dev we set the priority
>> value of the bio to the user supplied value.
>>
>> This patch depends on block: add ioprio_check_cap function
>
> Actually, one comment on this one:
>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index f3eae5d5771b..ff3107aa82d5 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1451,6 +1451,22 @@ static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
>> if (iocb->aio_flags & IOCB_FLAG_RESFD)
>> req->ki_flags |= IOCB_EVENTFD;
>> req->ki_hint = file_write_hint(req->ki_filp);
>> + if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
>> + /*
>> + * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
>> + * aio_reqprio is interpreted as an I/O scheduling
>> + * class and priority.
>> + */
>> + ret = ioprio_check_cap(iocb->aio_reqprio);
>> + if (ret) {
>> + pr_debug("aio ioprio check cap error\n");
>> + return -EINVAL;
>> + }
>> +
>> + req->ki_ioprio = iocb->aio_reqprio;
>> + req->ki_flags |= IOCB_IOPRIO;
>> + }
>
> Do we really need IOCB_IOPRIO? All zeroes is no priority set anyway,
> so we should be able to get by with just setting ->ki_ioprio to either
> the priority, or 0.
>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 7ec920e27065..970bef79caa6 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>> bio->bi_write_hint = iocb->ki_hint;
>> bio->bi_private = dio;
>> bio->bi_end_io = blkdev_bio_end_io;
>> + if (iocb->ki_flags & IOCB_IOPRIO)
>> + bio->bi_ioprio = iocb->ki_ioprio;
>
> And then this assignment can just happen unconditionally.

That is a cleaner way of guaranteeing the ioprio set on the kiocb is
only set when the user intends to use the ioprio from the iocb.

I'll resend the series.


>
\
 
 \ /
  Last update: 2018-05-18 17:30    [W:0.080 / U:8.368 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site