| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170523152651.GA7568@hgst.com>
Date: Tue, 23 May 2017 08:26:51 -0700
From: Adam Manzanares <adam.manzanares@....com>
To: Jan Kara <jack@...e.cz>
CC: <bcrl@...ck.org>, <viro@...iv.linux.org.uk>,
<jlayton@...chiereds.net>, <bfields@...ldses.org>,
<linux-aio@...ck.org>, <linux-fsdevel@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-api@...r.kernel.org>
Subject: Re: [RFC PATCH] fs: block dev aio request priority support
The 05/23/2017 10:46, Jan Kara wrote:
> On Mon 22-05-17 10:19:33, adam.manzanares@....com wrote:
> > From: Adam Manzanares <adam.manzanares@....com>
> >
> > Map the aio_reqprio to the bio priority field at
> > the point the bio is created from the aio iocb.
> >
> > The aio_reqprio field of iocb is used as a kernel IO class and priority
> > iff the IOCB_FLAG_IOPRIO flag is set on the iocb.
> >
> > Late last year device IO priority support was introduced to reduce application
> > tail latency when iopriority information was set on the process [1]. This
> > patch mapped iopriority information to block io requests. This information
> > could be leveraged by device drivers to build device specific prioritized
> > commands.
> >
> > The iopriority is set on the iocontext which is a structure associated with
> > a process. There exists a system call to set this iopriority information on
> > a process, but I believe it would be useful to also have a mechanism to set
> > priority on a per io command basis.
> >
> > The aio iocb has a field for the request priority which is currently not used
> > within the kernel. This patch leverages this field to pass a per command
> > iopriority value to devices. This work leverages the work in the previously
> > referenced patch [1]. When the bio is generated from the iocb we copy the
> > iocb iopriority information into the bio, which is eventually turned into a
> > request which also gets a copy of the iopriority information.
> >
> > To demonstrate how to use this feature I modified fio to use the new aio
> > feature. The modification to fio can be found at [2] and the new options
> > are cmndprioclass and cmndprio.
> >
> > [1] https://lkml.org/lkml/2016/12/6/495
> > [2] https://github.com/nmtadam/fio/tree/cmnd-prio.v2
> >
> > Signed-off-by: Adam Manzanares <adam.manzanares@....com>
>
> Using aio_flags is problematic because we never checked this for containing
> only expected flags. So userspace may be leaving this flag set
> unintentionally and currently it doesn't have any adverse effects. So it
> was decided to use a reserved word in struct iocb for new flags. And
> Goldwyn already did this as a part of his series [1] together with other IO
> flags. If you want, you can lobby for merging this particular patch earlier
> :).
Thanks for pointing this patch out, I missed it. I will base my work off of
this patch given that adding new flags to aio_flags is problematic.
>
> Honza
>
> [1] https://patchwork.kernel.org/patch/9722865/
>
>
> > ---
> > fs/aio.c | 9 +++++++++
> > fs/block_dev.c | 1 +
> > include/linux/fs.h | 1 +
> > include/uapi/linux/aio_abi.h | 6 ++++++
> > 4 files changed, 17 insertions(+)
> >
> > diff --git a/fs/aio.c b/fs/aio.c
> > index f52d925..a75a279 100644
> > --- a/fs/aio.c
> > +++ b/fs/aio.c
> > @@ -1568,6 +1568,15 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
> > req->common.ki_pos = iocb->aio_offset;
> > req->common.ki_complete = aio_complete;
> > req->common.ki_flags = iocb_flags(req->common.ki_filp);
> > + if (iocb->aio_flags & IOCB_FLAG_IOPRIO)
> > + /*
> > + * If the IOCB_FLAG_IOPRIO flag of aio_flags is set,
> > + * then the aio_reqprio is interpreted as a I/O
> > + * scheduling class and priority. This is then set
> > + * on the bio that is created from this request, which
> > + * enables the priority to be passed to device drivers.
> > + */
> > + req->common.ki_ioprio = iocb->aio_reqprio;
> >
> > if (iocb->aio_flags & IOCB_FLAG_RESFD) {
> > /*
> > diff --git a/fs/block_dev.c b/fs/block_dev.c
> > index 2eca00e..20d18db 100644
> > --- a/fs/block_dev.c
> > +++ b/fs/block_dev.c
> > @@ -360,6 +360,7 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
> > bio->bi_iter.bi_sector = pos >> 9;
> > bio->bi_private = dio;
> > bio->bi_end_io = blkdev_bio_end_io;
> > + bio->bi_ioprio = iocb->ki_ioprio;
> >
> > ret = bio_iov_iter_get_pages(bio, iter);
> > if (unlikely(ret)) {
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 866c955..83135f0 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -276,6 +276,7 @@ struct kiocb {
> > void (*ki_complete)(struct kiocb *iocb, long ret, long ret2);
> > void *private;
> > int ki_flags;
> > + u16 ki_ioprio; /* See linux/ioprio.h */
> > };
> >
> > static inline bool is_sync_kiocb(struct kiocb *kiocb)
> > diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> > index bb2554f..415980d 100644
> > --- a/include/uapi/linux/aio_abi.h
> > +++ b/include/uapi/linux/aio_abi.h
> > @@ -54,6 +54,12 @@ enum {
> > */
> > #define IOCB_FLAG_RESFD (1 << 0)
> >
> > +/*
> > + * IOCB_FLAG_IOPRIO - Set if the "aio_reqprio" member of the "struct iocb"
> > + * is interpreted as an I/O scheduling class and priority
> > + */
> > +#define IOCB_FLAG_IOPRIO (1 << 1)
> > +
> > /* read() from /dev/aio returns these structures. */
> > struct io_event {
> > __u64 data; /* the data field from the iocb */
> > --
> > 2.7.4
> >
> --
> Jan Kara <jack@...e.com>
> SUSE Labs, CR
Powered by blists - more mailing lists