* Re: [PATCH v5 1/5] block: add ioprio_check_cap function
From: Jeff Moyer @ 2018-05-21 20:58 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-2-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Aio per command iopriority support introduces a second interface between
> userland and the kernel capable of passing iopriority. The aio interface also
> needs the ability to verify that the submitting context has sufficient
> priviledges to submit IOPRIO_RT commands. This patch creates the
> ioprio_check_cap function to be used by the ioprio_set system call and also by
> the aio interface.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> block/ioprio.c | 22 ++++++++++++++++------
> include/linux/ioprio.h | 2 ++
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/block/ioprio.c b/block/ioprio.c
> index 6f5d0b6625e3..f9821080c92c 100644
> --- a/block/ioprio.c
> +++ b/block/ioprio.c
> @@ -61,15 +61,10 @@ int set_task_ioprio(struct task_struct *task, int ioprio)
> }
> EXPORT_SYMBOL_GPL(set_task_ioprio);
>
> -SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> +int ioprio_check_cap(int ioprio)
> {
> int class = IOPRIO_PRIO_CLASS(ioprio);
> int data = IOPRIO_PRIO_DATA(ioprio);
> - struct task_struct *p, *g;
> - struct user_struct *user;
> - struct pid *pgrp;
> - kuid_t uid;
> - int ret;
>
> switch (class) {
> case IOPRIO_CLASS_RT:
> @@ -92,6 +87,21 @@ SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> return -EINVAL;
> }
>
> + return 0;
> +}
> +
> +SYSCALL_DEFINE3(ioprio_set, int, which, int, who, int, ioprio)
> +{
> + struct task_struct *p, *g;
> + struct user_struct *user;
> + struct pid *pgrp;
> + kuid_t uid;
> + int ret;
> +
> + ret = ioprio_check_cap(ioprio);
> + if (ret)
> + return ret;
> +
> ret = -ESRCH;
> rcu_read_lock();
> switch (which) {
> diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h
> index 627efac73e6d..4a28cec49ec3 100644
> --- a/include/linux/ioprio.h
> +++ b/include/linux/ioprio.h
> @@ -77,4 +77,6 @@ extern int ioprio_best(unsigned short aprio, unsigned short bprio);
>
> extern int set_task_ioprio(struct task_struct *task, int ioprio);
>
> +extern int ioprio_check_cap(int ioprio);
> +
> #endif
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 3/5] fs: Add aio iopriority support
From: Jeff Moyer @ 2018-05-21 20:58 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-4-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> 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.
>
> We set the blkdev bio iopriority unconditionally, so we need to guarantee the
> kiocb is initialized properly. Added changes to the loopback driver and
> init_sync_kiocb to achieve this.
>
> This patch depends on block: add ioprio_check_cap function.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> drivers/block/loop.c | 3 +++
> fs/aio.c | 16 ++++++++++++++++
> include/linux/fs.h | 3 +++
> include/uapi/linux/aio_abi.h | 1 +
> 4 files changed, 23 insertions(+)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 5d4e31655d96..dd98dfd97f5e 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -76,6 +76,8 @@
> #include <linux/miscdevice.h>
> #include <linux/falloc.h>
> #include <linux/uio.h>
> +#include <linux/ioprio.h>
> +
> #include "loop.h"
>
> #include <linux/uaccess.h>
> @@ -559,6 +561,7 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> cmd->iocb.ki_filp = file;
> cmd->iocb.ki_complete = lo_rw_aio_complete;
> cmd->iocb.ki_flags = IOCB_DIRECT;
> + cmd->iocb.ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
> if (cmd->css)
> kthread_associate_blkcg(cmd->css);
>
> diff --git a/fs/aio.c b/fs/aio.c
> index f3eae5d5771b..44b4572be524 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;
> + } else
> + req->ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
> +
> ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
> if (unlikely(ret))
> fput(req->ki_filp);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 50de40dbbb85..73b749ed3ea1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -36,6 +36,7 @@
> #include <linux/delayed_call.h>
> #include <linux/uuid.h>
> #include <linux/errseq.h>
> +#include <linux/ioprio.h>
>
> #include <asm/byteorder.h>
> #include <uapi/linux/fs.h>
> @@ -302,6 +303,7 @@ struct kiocb {
> void *private;
> int ki_flags;
> u16 ki_hint;
> + u16 ki_ioprio; /* See linux/ioprio.h */
> } __randomize_layout;
>
> static inline bool is_sync_kiocb(struct kiocb *kiocb)
> @@ -1942,6 +1944,7 @@ static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
> .ki_filp = filp,
> .ki_flags = iocb_flags(filp),
> .ki_hint = ki_hint_validate(file_write_hint(filp)),
> + .ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0),
> };
> }
>
> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
> index 2c0a3415beee..d4e768d55d14 100644
> --- a/include/uapi/linux/aio_abi.h
> +++ b/include/uapi/linux/aio_abi.h
> @@ -55,6 +55,7 @@ enum {
> * is valid.
> */
> #define IOCB_FLAG_RESFD (1 << 0)
> +#define IOCB_FLAG_IOPRIO (1 << 1)
>
> /* read() from /dev/aio returns these structures. */
> struct io_event {
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 4/5] fs: blkdev set bio prio from kiocb prio
From: Jeff Moyer @ 2018-05-21 20:59 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-5-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Now that kiocb has an ioprio field copy this over to the bio when it is
> created from the kiocb.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> fs/block_dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7ec920e27065..da1e94d2bb75 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -355,6 +355,7 @@ __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;
> + bio->bi_ioprio = iocb->ki_ioprio;
>
> ret = bio_iov_iter_get_pages(bio, iter);
> if (unlikely(ret)) {
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 5/5] fs: iomap dio set bio prio from kiocb prio
From: Jeff Moyer @ 2018-05-21 20:59 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-6-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Now that kiocb has an ioprio field copy this over to the bio when it is
> created from the kiocb during direct IO.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
> ---
> fs/iomap.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/iomap.c b/fs/iomap.c
> index afd163586aa0..65aae194aeca 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -919,6 +919,7 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
> bio->bi_iter.bi_sector =
> (iomap->addr + pos - iomap->offset) >> 9;
> bio->bi_write_hint = dio->iocb->ki_hint;
> + bio->bi_ioprio = dio->iocb->ki_ioprio;
> bio->bi_private = dio;
> bio->bi_end_io = iomap_dio_bio_end_io;
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 4/5] fs: blkdev set bio prio from kiocb prio
From: Jeff Moyer @ 2018-05-21 21:04 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, hch
In-Reply-To: <20180521160147.2372-5-adam.manzanares@wdc.com>
adam.manzanares@wdc.com writes:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Now that kiocb has an ioprio field copy this over to the bio when it is
> created from the kiocb.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
> ---
> fs/block_dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7ec920e27065..da1e94d2bb75 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -355,6 +355,7 @@ __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;
> + bio->bi_ioprio = iocb->ki_ioprio;
>
> ret = bio_iov_iter_get_pages(bio, iter);
> if (unlikely(ret)) {
Forgot to mention, you should also convert __blkdev_direct_IO_simple.
-Jeff
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 0/5] AIO add per-command iopriority
From: Adam Manzanares @ 2018-05-21 21:42 UTC (permalink / raw)
To: Jeff Moyer
Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
axboe@kernel.dk, bcrl@kvack.org, tglx@linutronix.de,
mingo@kernel.org, pombredanne@nexb.com,
kstewart@linuxfoundation.org, gregkh@linuxfoundation.org,
bigeasy@linutronix.de, jack@suse.cz, darrick.wong@oracle.com,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org, hch@infradead.org
In-Reply-To: <x49fu2kvi9q.fsf@segfault.boston.devel.redhat.com>
On 5/21/18 1:57 PM, Jeff Moyer wrote:
> Hi, Adam,
>
> adam.manzanares@wdc.com writes:
>
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> This is the per-I/O equivalent of the ioprio_set system call.
>> See the following link for performance implications on a SATA HDD:
>> https://lkml.org/lkml/2016/12/6/495
>>
>> First patch factors ioprio_check_cap function out of ioprio_set system call to
>> also be used by the aio ioprio interface.
>>
>> Second patch converts kiocb ki_hint field to a u16 to avoid kiocb bloat.
>>
>> Third patch passes ioprio hint from aio iocb to kiocb and inititalizes kiocb
>> ioprio value appropriately when it is not explicitly set.
>>
>> Fourth patch enables the feature for blkdev.
>>
>> Fifth patch enables the feature for iomap direct IO
>>
>> Note: this work is based on top of linux-vfs/for-next
>
> I'll cook up a libaio test case. Can you put together a man-pages
> update for this?
Hello Jeff,
I'll send out a man-pages update soon.
Thanks,
Adam
>
> Thanks!
> Jeff
>
^ permalink raw reply
* Re: [PATCH v5 4/5] fs: blkdev set bio prio from kiocb prio
From: Adam Manzanares @ 2018-05-21 21:43 UTC (permalink / raw)
To: Jeff Moyer
Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
axboe@kernel.dk, bcrl@kvack.org, tglx@linutronix.de,
mingo@kernel.org, pombredanne@nexb.com,
kstewart@linuxfoundation.org, gregkh@linuxfoundation.org,
bigeasy@linutronix.de, jack@suse.cz, darrick.wong@oracle.com,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org, hch@infradead.org
In-Reply-To: <x49r2m4u3e6.fsf@segfault.boston.devel.redhat.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1074 bytes --]
On 5/21/18 2:04 PM, Jeff Moyer wrote:
> adam.manzanares@wdc.com writes:
>
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> Now that kiocb has an ioprio field copy this over to the bio when it is
>> created from the kiocb.
>>
>> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
>> ---
>> fs/block_dev.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 7ec920e27065..da1e94d2bb75 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -355,6 +355,7 @@ __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;
>> + bio->bi_ioprio = iocb->ki_ioprio;
>>
>> ret = bio_iov_iter_get_pages(bio, iter);
>> if (unlikely(ret)) {
>
> Forgot to mention, you should also convert __blkdev_direct_IO_simple.
NP.
>
> -Jeff
> N‹§²æìr¸›zǧu©ž²Æ {\b†éì¹»\x1c®&Þ–)îŨ¨Š{ayº\x1dÊÚ&j:+v‰¨’öœ’Šà\x16Šæ¢·¢ú(œ¸§»\x10\b:Çž†Ûiÿü0ÂKÚrJ+ƒö¢£ðèž×¦j)Z†·Ÿ
^ permalink raw reply
* Re: [PATCH v2 2/4] mm: check for proper migrate type during isolation
From: Mike Kravetz @ 2018-05-21 23:10 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton, Joonsoo Kim
In-Reply-To: <0a74f688-74fb-b841-4782-f9c96b1b9cfc@suse.cz>
On 05/18/2018 03:32 AM, Vlastimil Babka wrote:
> On 05/04/2018 01:29 AM, Mike Kravetz wrote:
>> The routine start_isolate_page_range and alloc_contig_range have
>> comments saying that migratetype must be either MIGRATE_MOVABLE or
>> MIGRATE_CMA. However, this is not enforced.
>
> Enforced, no. But if the pageblocks really were as such, it used to
> shortcut has_unmovable_pages(). This was wrong and removed in
> d7b236e10ced ("mm: drop migrate type checks from has_unmovable_pages")
> plus 4da2ce250f98 ("mm: distinguish CMA and MOVABLE isolation in
> has_unmovable_pages()").
>
>
> What is important is
>> that that all pageblocks in the range are of type migratetype.
> the same
>> This is because blocks will be set to migratetype on error.
>
> Strictly speaking this is true only for the CMA case. For other cases,
> the best thing actually would be to employ the same heuristics as page
> allocation migratetype fallbacks, and count how many pages are free and
> how many appear to be movable, see how steal_suitable_fallback() uses
> the last parameter of move_freepages_block().
>
>> Add a boolean argument enforce_migratetype to the routine
>> start_isolate_page_range. If set, it will check that all pageblocks
>> in the range have the passed migratetype. Return -EINVAL is pageblock
> if
>> is wrong type is found in range.
> of
>>
>> A boolean is used for enforce_migratetype as there are two primary
>> users. Contiguous range allocation which wants to enforce migration
>> type checking. Memory offline (hotplug) which is not concerned about
>> type checking.
>
> This is missing some high-level result. The end change is that CMA is
> now enforcing. So we are making it more robust when it's called on
> non-CMA pageblocks by mistake? (BTW I still do hope we can remove
> MIGRATE_CMA soon after Joonsoo's ZONE_MOVABLE CMA conversion. Combined
> with my suggestion above we could hopefully get rid of the migratetype
> parameter completely instead of enforcing it?). Is this also a
> preparation for introducing find_alloc_contig_pages() which will be
> enforcing? (I guess, and will find out shortly, but it should be stated
> here)
Thank you for looking at these patches Vlastimil.
My primary motivation for this patch was the 'error recovery' in
start_isolate_page_range. It takes a range and attempts to set
all pageblocks to MIGRATE_ISOLATE. If it encounters an error after
setting some blocks to isolate, it will 'clean up' by setting the
migrate type of previously modified blocks to the passed migratetype.
So, one possible side effect of an error in start_isolate_page_range
is that the migrate type of some pageblocks could be modified. Thinking
about it more now, that may be OK. It just does not seem like the
right thing to do, especially with comments saying "migratetype must
be either MIGRATE_MOVABLE or MIGRATE_CMA". I'm fine with leaving the
code as is and just cleaning up the comments if you think that may
be better.
>
> Thanks,
> Vlastimil
>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>> include/linux/page-isolation.h | 8 +++-----
>> mm/memory_hotplug.c | 2 +-
>> mm/page_alloc.c | 17 +++++++++--------
>> mm/page_isolation.c | 40 ++++++++++++++++++++++++++++++----------
>> 4 files changed, 43 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>> index 4ae347cbc36d..2ab7e5a399ac 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -38,8 +38,6 @@ int move_freepages_block(struct zone *zone, struct page *page,
>>
>> /*
>> * Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.
>> - * If specified range includes migrate types other than MOVABLE or CMA,
>> - * this will fail with -EBUSY.
>> *
>> * For isolating all pages in the range finally, the caller have to
>> * free all pages in the range. test_page_isolated() can be used for
>> @@ -47,11 +45,11 @@ int move_freepages_block(struct zone *zone, struct page *page,
>> */
>> int
>> start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> - unsigned migratetype, bool skip_hwpoisoned_pages);
>> + unsigned migratetype, bool skip_hwpoisoned_pages,
>> + bool enforce_migratetype);
>>
>> /*
>> - * Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
>> - * target range is [start_pfn, end_pfn)
>> + * Changes MIGRATE_ISOLATE to migratetype for range [start_pfn, end_pfn)
>> */
>> int
>> undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index f74826cdceea..ebc1c8c330e2 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1601,7 +1601,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
>>
>> /* set above range as isolated */
>> ret = start_isolate_page_range(start_pfn, end_pfn,
>> - MIGRATE_MOVABLE, true);
>> + MIGRATE_MOVABLE, true, false);
>> if (ret)
>> return ret;
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 0fd5e8e2456e..cb1a5e0be6ee 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7787,9 +7787,10 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
>> * alloc_contig_range() -- tries to allocate given range of pages
>> * @start: start PFN to allocate
>> * @end: one-past-the-last PFN to allocate
>> - * @migratetype: migratetype of the underlaying pageblocks (either
>> - * #MIGRATE_MOVABLE or #MIGRATE_CMA). All pageblocks
>> - * in range must have the same migratetype and it must
>> + * @migratetype: migratetype of the underlaying pageblocks. All
>> + * pageblocks in range must have the same migratetype.
>> + * migratetype is typically MIGRATE_MOVABLE or
>> + * MIGRATE_CMA, but this is not a requirement.
>> * be either of the two.
>> * @gfp_mask: GFP mask to use during compaction
>> *
>> @@ -7840,15 +7841,15 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>> * allocator removing them from the buddy system. This way
>> * page allocator will never consider using them.
>> *
>> - * This lets us mark the pageblocks back as
>> - * MIGRATE_CMA/MIGRATE_MOVABLE so that free pages in the
>> - * aligned range but not in the unaligned, original range are
>> - * put back to page allocator so that buddy can use them.
>> + * This lets us mark the pageblocks back as their original
>> + * migrate type so that free pages in the aligned range but
>> + * not in the unaligned, original range are put back to page
>> + * allocator so that buddy can use them.
>> */
>>
>> ret = start_isolate_page_range(pfn_max_align_down(start),
>> pfn_max_align_up(end), migratetype,
>> - false);
>> + false, true);
>> if (ret)
>> return ret;
>>
>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> index 43e085608846..472191cc1909 100644
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -16,7 +16,8 @@
>> #include <trace/events/page_isolation.h>
>>
>> static int set_migratetype_isolate(struct page *page, int migratetype,
>> - bool skip_hwpoisoned_pages)
>> + bool skip_hwpoisoned_pages,
>> + bool enforce_migratetype)
>> {
>> struct zone *zone;
>> unsigned long flags, pfn;
>> @@ -36,6 +37,17 @@ static int set_migratetype_isolate(struct page *page, int migratetype,
>> if (is_migrate_isolate_page(page))
>> goto out;
>>
>> + /*
>> + * If requested, check migration type of pageblock and make sure
>> + * it matches migratetype
>> + */
>> + if (enforce_migratetype) {
>> + if (get_pageblock_migratetype(page) != migratetype) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + }
>> +
>> pfn = page_to_pfn(page);
>> arg.start_pfn = pfn;
>> arg.nr_pages = pageblock_nr_pages;
>> @@ -167,14 +179,16 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
>> * to be MIGRATE_ISOLATE.
>> * @start_pfn: The lower PFN of the range to be isolated.
>> * @end_pfn: The upper PFN of the range to be isolated.
>> - * @migratetype: migrate type to set in error recovery.
>> + * @migratetype: migrate type of all blocks in range.
>> *
>> * Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
>> * the range will never be allocated. Any free pages and pages freed in the
>> * future will not be allocated again.
>> *
>> * start_pfn/end_pfn must be aligned to pageblock_order.
>> - * Return 0 on success and -EBUSY if any part of range cannot be isolated.
>> + * Return 0 on success or error returned by set_migratetype_isolate. Typical
>> + * errors are -EBUSY if any part of range cannot be isolated or -EINVAL if
>> + * any page block is not of migratetype.
>> *
>> * There is no high level synchronization mechanism that prevents two threads
>> * from trying to isolate overlapping ranges. If this happens, one thread
>> @@ -185,11 +199,13 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
>> * prevents two threads from simultaneously working on overlapping ranges.
>> */
>> int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> - unsigned migratetype, bool skip_hwpoisoned_pages)
>> + unsigned migratetype, bool skip_hwpoisoned_pages,
>> + bool enforce_migratetype)
>> {
>> unsigned long pfn;
>> unsigned long undo_pfn;
>> struct page *page;
>> + int ret = 0;
>>
>> BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
>> BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
>> @@ -198,13 +214,17 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> pfn < end_pfn;
>> pfn += pageblock_nr_pages) {
>> page = __first_valid_page(pfn, pageblock_nr_pages);
>> - if (page &&
>> - set_migratetype_isolate(page, migratetype, skip_hwpoisoned_pages)) {
>> - undo_pfn = pfn;
>> - goto undo;
>> + if (page) {
>> + ret = set_migratetype_isolate(page, migratetype,
>> + skip_hwpoisoned_pages,
>> + enforce_migratetype);
>> + if (ret) {
>> + undo_pfn = pfn;
>> + goto undo;
>> + }
>> }
>> }
>> - return 0;
>> + return ret;
>> undo:
>> for (pfn = start_pfn;
>> pfn < undo_pfn;
>> @@ -215,7 +235,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> unset_migratetype_isolate(page, migratetype);
>> }
>>
>> - return -EBUSY;
>> + return ret;
>> }
>>
>> /*
>>
>
--
Mike Kravetz
^ permalink raw reply
* Re: [PATCH v2 3/4] mm: add find_alloc_contig_pages() interface
From: Mike Kravetz @ 2018-05-21 23:48 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <eaa40ac0-365b-fd27-e096-b171ed28888f@suse.cz>
On 05/21/2018 01:54 AM, Vlastimil Babka wrote:
> On 05/04/2018 01:29 AM, Mike Kravetz wrote:
>> find_alloc_contig_pages() is a new interface that attempts to locate
>> and allocate a contiguous range of pages. It is provided as a more
>
> How about dropping the 'find_' from the name, so it's more like other
> allocator functions? All of them have to 'find' the free pages in some
> sense.
Sure
>
>> convenient interface than alloc_contig_range() which is currently
>> used by CMA and gigantic huge pages.
>>
>> When attempting to allocate a range of pages, migration is employed
>> if possible. There is no guarantee that the routine will succeed.
>> So, the user must be prepared for failure and have a fall back plan.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>> include/linux/gfp.h | 12 +++++
>> mm/page_alloc.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 146 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index 86a0d06463ab..b0d11777d487 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -573,6 +573,18 @@ static inline bool pm_suspended_storage(void)
>> extern int alloc_contig_range(unsigned long start, unsigned long end,
>> unsigned migratetype, gfp_t gfp_mask);
>> extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
>> +extern struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
>> + int nid, nodemask_t *nodemask);
>> +extern void free_contig_pages(struct page *page, unsigned long nr_pages);
>> +#else
>> +static inline struct page *find_alloc_contig_pages(unsigned long nr_pages,
>> + gfp_t gfp, int nid, nodemask_t *nodemask)
>> +{
>> + return NULL;
>> +}
>> +static inline void free_contig_pages(struct page *page, unsigned long nr_pages)
>> +{
>> +}
>> #endif
>>
>> #ifdef CONFIG_CMA
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index cb1a5e0be6ee..d0a2d0da9eae 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -67,6 +67,7 @@
>> #include <linux/ftrace.h>
>> #include <linux/lockdep.h>
>> #include <linux/nmi.h>
>> +#include <linux/mmzone.h>
>>
>> #include <asm/sections.h>
>> #include <asm/tlbflush.h>
>> @@ -7913,8 +7914,12 @@ int alloc_contig_range(unsigned long start, unsigned long end,
>>
>> /* Make sure the range is really isolated. */
>> if (test_pages_isolated(outer_start, end, false)) {
>> - pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
>> - __func__, outer_start, end);
>> +#ifdef MIGRATE_CMA
>> + /* Only print messages for CMA allocations */
>> + if (migratetype == MIGRATE_CMA)
>
> I think is_migrate_cma() can be used to avoid the #ifdef.
>
Thanks. I missed that and did not want to create something new.
>> + pr_info_ratelimited("%s: [%lx, %lx) PFNs busy\n",
>> + __func__, outer_start, end);
>> +#endif
>> ret = -EBUSY;
>> goto done;
>> }
>> @@ -7950,6 +7955,133 @@ void free_contig_range(unsigned long pfn, unsigned long nr_pages)
>> }
>> WARN(count != 0, "%ld pages are still in use!\n", count);
>> }
>> +
>> +/*
>> + * Only check for obvious pfn/pages which can not be used/migrated. The
>> + * migration code will do the final check. Under stress, this minimal set
>> + * has been observed to provide the best results. The checks can be expanded
>> + * if needed.
>
> Hm I kind of doubt this is optimal, it doesn't test almost anything
> besides basic validity, so it won't exclude ranges where the allocation
> will fail. I will write more in a reply to the header where complexity
> is discussed.
>
Ok. This 'appeared' to work best in testing where I had all CPUs in tight
loops calling this new interface to allocate and then free contiguous pages.
I was somewhat surprised at the result, and it may just be due to the nature
of my testing.
>> + */
>> +static bool contig_pfn_range_valid(struct zone *z, unsigned long start_pfn,
>> + unsigned long nr_pages)
>> +{
>> + unsigned long i, end_pfn = start_pfn + nr_pages;
>> + struct page *page;
>> +
>> + for (i = start_pfn; i < end_pfn; i++) {
>> + if (!pfn_valid(i))
>> + return false;
>> +
>> + page = pfn_to_online_page(i);
>> +
>> + if (page_zone(page) != z)
>> + return false;
>> +
>> + }
>> +
>> + return true;
>> +}
>> +
>> +/*
>> + * Search for and attempt to allocate contiguous allocations greater than
>> + * MAX_ORDER.
>> + */
>> +static struct page *__alloc_contig_pages_nodemask(gfp_t gfp,
>> + unsigned long order,
>> + int nid, nodemask_t *nodemask)
>> +{
>> + unsigned long nr_pages, pfn, flags;
>> + struct page *ret_page = NULL;
>> + struct zonelist *zonelist;
>> + struct zoneref *z;
>> + struct zone *zone;
>> + int rc;
>> +
>> + nr_pages = 1 << order;
>> + zonelist = node_zonelist(nid, gfp);
>> + for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp),
>> + nodemask) {
>> + pgdat_resize_lock(zone->zone_pgdat, &flags);
>> + pfn = ALIGN(zone->zone_start_pfn, nr_pages);
>> + while (zone_spans_pfn(zone, pfn + nr_pages - 1)) {
>> + if (contig_pfn_range_valid(zone, pfn, nr_pages)) {
>> + struct page *page = pfn_to_online_page(pfn);
>> + unsigned int migratetype;
>> +
>> + /*
>> + * All pageblocks in range must be of same
>> + * migrate type.
>> + */
>> + migratetype = get_pageblock_migratetype(page);
>> + pgdat_resize_unlock(zone->zone_pgdat, &flags);
>> +
>> + rc = alloc_contig_range(pfn, pfn + nr_pages,
>> + migratetype, gfp);
>> + if (!rc) {
>> + ret_page = pfn_to_page(pfn);
>> + return ret_page;
>> + }
>> + pgdat_resize_lock(zone->zone_pgdat, &flags);
>> + }
>> + pfn += nr_pages;
>> + }
>> + pgdat_resize_unlock(zone->zone_pgdat, &flags);
>> + }
>> +
>> + return ret_page;
>> +}
>> +
>> +/**
>> + * find_alloc_contig_pages() -- attempt to find and allocate a contiguous
>> + * range of pages
>> + * @nr_pages: number of pages to find/allocate
>> + * @gfp: gfp mask used to limit search as well as during compaction
>> + * @nid: target node
>> + * @nodemask: mask of other possible nodes
>> + *
>> + * Pages can be freed with a call to free_contig_pages(), or by manually
>> + * calling __free_page() for each page allocated.
>> + *
>> + * Return: pointer to 'order' pages on success, or NULL if not successful.
>> + */
>> +struct page *find_alloc_contig_pages(unsigned long nr_pages, gfp_t gfp,
>> + int nid, nodemask_t *nodemask)
>> +{
>> + unsigned long i, alloc_order, order_pages;
>> + struct page *pages;
>> +
>> + /*
>> + * Underlying allocators perform page order sized allocations.
>> + */
>> + alloc_order = get_count_order(nr_pages);
>
> So if takes arbitrary nr_pages but convert it to order anyway? I think
> that's rather suboptimal and wasteful... e.g. a range could be skipped
> because some of the pages added by rounding cannot be migrated away.
Yes. My idea with this series was to use existing allocators which are
all order based. Let me think about how to do allocation for arbitrary
number of allocations.
- For less than MAX_ORDER size we rely on the buddy allocator, so we are
pretty much stuck with order sized allocation. However, allocations of
this size are not really interesting as you can call existing routines
directly.
- For sizes greater than MAX_ORDER, we know that the allocation size will
be at least pageblock sized. So, the isolate/migrate scheme can still
be used for full pageblocks. We can then use direct migration for the
remaining pages. This does complicate things a bit.
I'm guessing that most (?all?) allocations will be order based. The use
cases I am aware of (hugetlbfs, Intel Cache Pseudo-Locking, RDMA) are all
order based. However, as commented in previous version taking arbitrary
nr_pages makes interface more future proof.
--
Mike Kravetz
>
> Vlastimil
>
>> + if (alloc_order < MAX_ORDER) {
>> + pages = __alloc_pages_nodemask(gfp, (unsigned int)alloc_order,
>> + nid, nodemask);
>> + split_page(pages, alloc_order);
>> + } else {
>> + pages = __alloc_contig_pages_nodemask(gfp, alloc_order, nid,
>> + nodemask);
>> + }
>> +
>> + if (pages) {
>> + /*
>> + * More pages than desired could have been allocated due to
>> + * rounding up to next page order. Free any excess pages.
>> + */
>> + order_pages = 1UL << alloc_order;
>> + for (i = nr_pages; i < order_pages; i++)
>> + __free_page(pages + i);
>> + }
>> +
>> + return pages;
>> +}
>> +EXPORT_SYMBOL_GPL(find_alloc_contig_pages);
>> +
>> +void free_contig_pages(struct page *page, unsigned long nr_pages)
>> +{
>> + free_contig_range(page_to_pfn(page), nr_pages);
>> +}
>> +EXPORT_SYMBOL_GPL(free_contig_pages);
>> #endif
>>
>> #if defined CONFIG_MEMORY_HOTPLUG || defined CONFIG_CMA
>>
>
^ permalink raw reply
* Re: [PATCH v2 0/4] Interface for higher order contiguous allocations
From: Mike Kravetz @ 2018-05-22 0:15 UTC (permalink / raw)
To: Vlastimil Babka, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton
In-Reply-To: <8ce9884c-36b0-68ea-45a4-06177c41af4a@suse.cz>
On 05/21/2018 05:00 AM, Vlastimil Babka wrote:
> On 05/04/2018 01:29 AM, Mike Kravetz wrote:
>> Vlastimil and Michal brought up the issue of allocation alignment. The
>> routine will currently align to 'nr_pages' (which is the requested size
>> argument). It does this by examining and trying to allocate the first
>> nr_pages aligned/nr_pages sized range. If this fails, it moves on to the
>> next nr_pages aligned/nr_pages sized range until success or all potential
>> ranges are exhausted.
>
> As I've noted in my patch 3/4 review, in fact nr_pages is first rounded
> up to an order, which makes this simpler, but suboptimal. I think we
> could perhaps assume that nr_pages that's a power of two should be
> aligned as such, and other values of nr_pages need no alignment? This
> should fit existing users, and can be extended to explicit alignment
> when such user appears?
I'm good with that. I do believe that minimum alignment will be
pageblock size alignment (for > MAX_ORDER allocations).
>> If we allow an alignment to be specified, we will
>> need to potentially check all alignment aligned/nr_pages sized ranges.
>> In the worst case where alignment = PAGE_SIZE, this could result in huge
>> increase in the number of ranges to check.
>> To help cut down on the number of ranges to check, we could identify the
>> first page that causes a range allocation failure and start the next
>> range at the next aligned boundary. I tried this, and we still end up
>> with a huge number of ranges and wasted CPU cycles.
>
> I think the wasted cycle issues is due to the current code structure,
> which is based on the CMA use-case, which assumes that the allocations
> will succeed, because the areas are reserved and may contain only
> movable allocations
>
> find_alloc_contig_pages()
> __alloc_contig_pages_nodemask()
> contig_pfn_range_valid()
> - performs only very basic pfn validity and belongs-to-zone checks
> alloc_contig_range()
> start_isolate_page_range()
> for (pfn per pageblock) - the main cycle
> set_migratetype_isolate()
> has_unmovable_pages() - cancel if yes
> move_freepages_block() - expensive!
> __alloc_contig_migrate_range()
> etc (not important)
>
> So I think the problem is that in the main cycle we might do a number of
> expensive move_freepages_block() operations, then hit a block where
> has_unmovable_pages() is true, cancel and do more expensive
> undo_isolate_page_range() operations.
>
> If we instead first scanned the range with has_unmovable_pages() and
> only start doing the expensive work when we find a large enough (aligned
> or not depending on caller) range, it should be much faster and there
> should be no algorithmic difference between aligned and non-aligned case.
Ok, I will give that a try.
Thanks again for looking at these.
--
Mike Kravetz
^ permalink raw reply
* Re: [PATCH v2 2/4] mm: check for proper migrate type during isolation
From: Vlastimil Babka @ 2018-05-22 7:07 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton, Joonsoo Kim
In-Reply-To: <f50d6814-8bc6-80cd-c0e5-b2cfa4f9e576@oracle.com>
On 05/22/2018 01:10 AM, Mike Kravetz wrote:
> On 05/18/2018 03:32 AM, Vlastimil Babka wrote:
>> On 05/04/2018 01:29 AM, Mike Kravetz wrote:
>>> The routine start_isolate_page_range and alloc_contig_range have
>>> comments saying that migratetype must be either MIGRATE_MOVABLE or
>>> MIGRATE_CMA. However, this is not enforced.
>>
>> Enforced, no. But if the pageblocks really were as such, it used to
>> shortcut has_unmovable_pages(). This was wrong and removed in
>> d7b236e10ced ("mm: drop migrate type checks from has_unmovable_pages")
>> plus 4da2ce250f98 ("mm: distinguish CMA and MOVABLE isolation in
>> has_unmovable_pages()").
>>
>>
>> What is important is
>>> that that all pageblocks in the range are of type migratetype.
>> the same
>>> This is because blocks will be set to migratetype on error.
>>
>> Strictly speaking this is true only for the CMA case. For other cases,
>> the best thing actually would be to employ the same heuristics as page
>> allocation migratetype fallbacks, and count how many pages are free and
>> how many appear to be movable, see how steal_suitable_fallback() uses
>> the last parameter of move_freepages_block().
>>
>>> Add a boolean argument enforce_migratetype to the routine
>>> start_isolate_page_range. If set, it will check that all pageblocks
>>> in the range have the passed migratetype. Return -EINVAL is pageblock
>> if
>>> is wrong type is found in range.
>> of
>>>
>>> A boolean is used for enforce_migratetype as there are two primary
>>> users. Contiguous range allocation which wants to enforce migration
>>> type checking. Memory offline (hotplug) which is not concerned about
>>> type checking.
>>
>> This is missing some high-level result. The end change is that CMA is
>> now enforcing. So we are making it more robust when it's called on
>> non-CMA pageblocks by mistake? (BTW I still do hope we can remove
>> MIGRATE_CMA soon after Joonsoo's ZONE_MOVABLE CMA conversion. Combined
>> with my suggestion above we could hopefully get rid of the migratetype
>> parameter completely instead of enforcing it?). Is this also a
>> preparation for introducing find_alloc_contig_pages() which will be
>> enforcing? (I guess, and will find out shortly, but it should be stated
>> here)
>
> Thank you for looking at these patches Vlastimil.
>
> My primary motivation for this patch was the 'error recovery' in
> start_isolate_page_range. It takes a range and attempts to set
> all pageblocks to MIGRATE_ISOLATE. If it encounters an error after
> setting some blocks to isolate, it will 'clean up' by setting the
> migrate type of previously modified blocks to the passed migratetype.
Right.
> So, one possible side effect of an error in start_isolate_page_range
> is that the migrate type of some pageblocks could be modified. Thinking
> about it more now, that may be OK.
It would be definitely OK if the migratetype was changed similarly as
steal_suitable_fallback() does it, as I've said above.
> It just does not seem like the
> right thing to do, especially with comments saying "migratetype must
> be either MIGRATE_MOVABLE or MIGRATE_CMA". I'm fine with leaving the
> code as is and just cleaning up the comments if you think that may
> be better.
That's also possible, especially when the code is restructured as I've
suggested in the other reply, which should significantly reduce the
amount of error recoveries.
^ permalink raw reply
* Re: [PATCH v5 04/28] fpga: mgr: add compat_id support
From: Wu Hao @ 2018-05-22 9:39 UTC (permalink / raw)
To: Alan Tull
Cc: Moritz Fischer, linux-fpga, linux-kernel, linux-api, Kang, Luwei,
Zhang, Yi Z, Liu, Song
In-Reply-To: <CANk1AXTDgVONiaECNOnwj26vCxJSi7ixvD_QcB4VmEo7P_cUMA@mail.gmail.com>
On Mon, May 21, 2018 at 12:33:05PM -0500, Alan Tull wrote:
> On Sun, May 20, 2018 at 10:03 PM, Wu Hao <hao.wu@intel.com> wrote:
> > On Mon, May 07, 2018 at 04:09:06PM -0500, Alan Tull wrote:
> >> On Tue, May 1, 2018 at 9:50 PM, Wu Hao <hao.wu@intel.com> wrote:
> >>
> >> Hi Hao,
> >>
> >> Looks good!
> >>
> >> > This patch introduces compat_id support to fpga manager, it adds
> >> > a fpga_compat_id pointer to fpga manager data structure to allow
> >> > fpga manager drivers to save the compatibility id. This compat_id
> >> > could be used for compatibility checking before doing partial
> >> > reconfiguration to associated fpga regions.
> >> >
> >> > Signed-off-by: Wu Hao <hao.wu@intel.com>
> >> Acked-by: Alan Tull <atull@kernel.org>
> >
> > Hi Alan
> >
> > Thanks a lot for the acked-by.
> >
> > Did you get a chance to look into other patches?
>
> What I'm looking for mostly is: is it clear that this code was written
> to be reused. What do you think? Was it? Is there a way that intent
> could be made clear in the code?
Hi Alan,
I am a little confused, so I guess I need to clarify several things here
to avoid misunderstanding.
Actualy we are submitting this patchset to enable Intel FPGA products
(e.g Intel Programmable Acceleration Card (PAC) with Intel Arria 10 GX
FPGA[1]). Once this device driver is accepted by upstream, then people
could use them with standard linux kernel. So the major purpose of the
patchset is just a device driver for Intel specific FPGA device enabling.
The Device Feature List (DFL) is one concept used in the Intel PAC A10
FPGA device, it's a linked list of device features with predefined data
structures. The DFL is defined in the FPGA device spec from Intel, but we
all think it would be great if more people coud reuse it in their devices
too, so some code could be reused. I think it may not be a problem for
some of other Intel FPGA products to reuse these driver code, because they
probably follow the same DFL spec to have Port and FME implemented. It's
the same for other developers/vendors to reuse DFL to create their own
products. They have to implement the Port and FME in device memory per
spec, otherwise it may not be able to reuse current patchset. Please note
that current DFL spec doesn't provide a method to customize the Port or
FME, or even have a totally new Port or FME, but it's possible in the
future version of DFL.
So to me, it's conditional reusable for current patchset. It requires the
FPGA device to follow the same DFL spec with Port and FME defined (and
their private features too).
> This patchset has a history of being
> a one-off solution for a single platform, doing things to work around
> the FPGA framework instead of doing what the framework was intended to
> do. The FPGA framework was written so that any FPGA could be used
> with interface. Currently in the upstream that means any of the
> supported FPGAs can be programmed with the same of-fpga-region code.
> It didn't have to get rewritten for each fpga.
Per your suggestion, we already have a separated layer for enumeration,
different platform devices/drivers for different functional blocks (e.g
Port and FME), it also creates fpga region, manager, bridge to keep aligned
with current FPGA framework. Thanks again for your valuable suggestions.
>
> This patchset adds 5000 lines and I understand that another 4000 is
> coming to add to this.
This is because there are a lot of features implemented by Intel FPGA
products (e.g Intel PAC Card). Most code after this patchset is to add
private features support to Port and FME.
> Has that been written so that the upper layer
> can be reused? Or will the 'reusable' version be another huge
> patchset? Do you see my point?
I think current code is conditional reusable per explaination above, do
you agree? or you have other concerns on current driver architecture?
> Up to this point I've been trying to
> help figure out what changes could make this reusable. If you could
> get with someone to take responsibility for architecting this patchset
> to be clearly reusable, that could speed things up.
Please let me know which part is unclear to you. I see you use "clear"
for several times in your response, I do feel you have some concerns
somewhere, may I know which part is not clear to you? If you're not clear
about how DFL could support the customization. e.g if someone introduced
a totally new Port, then how it's going to reuse our current driver. I
would like to say, the behavior is not defined by DFL spec at all, so
current code may not be able to reused at all. Actually there are a lot
of similar open questions which are also unclear to me (e.g if spec will
allow to introduce a totally new port, if yes, then how to handle the
reset code with existing FME) as spec says nothing about these cases, at
least for current version. As no related description in DFL spec, then
we don't have to consider these cases for now.
Thanks
Hao
>
> If there are improvements to the current FPGA framework that can help
> this work, I'm interested and open to suggestions/code in that
> direction..
>
> Alan
>
> >
> > Thanks
> > Hao
^ permalink raw reply
* Re: [PATCH v5 2/5] fs: Convert kiocb rw_hint from enum to u16
From: Christoph Hellwig @ 2018-05-22 10:04 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api
In-Reply-To: <20180521160147.2372-3-adam.manzanares@wdc.com>
On Mon, May 21, 2018 at 09:01:44AM -0700, adam.manzanares@wdc.com wrote:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> In order to avoid kiocb bloat for per command iopriority support, rw_hint
> is converted from enum to a u16. Added a guard around ki_hint assigment.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 3/5] fs: Add aio iopriority support
From: Christoph Hellwig @ 2018-05-22 10:05 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api
In-Reply-To: <20180521160147.2372-4-adam.manzanares@wdc.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v5 5/5] fs: iomap dio set bio prio from kiocb prio
From: Christoph Hellwig @ 2018-05-22 10:05 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, tglx, mingo, pombredanne,
kstewart, gregkh, bigeasy, jack, darrick.wong, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api
In-Reply-To: <20180521160147.2372-6-adam.manzanares@wdc.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* aio poll and a new in-kernel poll API V12
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
Hi all,
this series adds support for the IOCB_CMD_POLL operation to poll for the
readyness of file descriptors using the aio subsystem. The API is based
on patches that existed in RHAS2.1 and RHEL3, which means it already is
supported by libaio. To implement the poll support efficiently new
methods to poll are introduced in struct file_operations: get_poll_head
and poll_mask. The first one returns a wait_queue_head to wait on
(lifetime is bound by the file), and the second does a non-blocking
check for the POLL* events. This allows aio poll to work without
any additional context switches, unlike epoll.
This series sits on top of the aio-fsync series that also includes
support for io_pgetevents.
The changes were sponsored by Scylladb, and improve performance
of the seastar framework up to 10%, while also removing the need
for a privileged SCHED_FIFO epoll listener thread.
git://git.infradead.org/users/hch/vfs.git aio-poll.12
Gitweb:
http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/aio-poll.12
Libaio changes:
https://pagure.io/libaio.git io-poll
Seastar changes (not updated for the new io_pgetevens ABI yet):
https://github.com/avikivity/seastar/commits/aio
Changes since v11:
- simplify cancellation by completion poll requests from a workqueue
if we can't take the ctx_lock
Changes since v10:
- fixed a mismerge that let a sock_rps_record_flow sneak into
tcp_poll_mask
- remove the now unused struct proto_ops get_poll_head method
Changes since v9:
- add to the delayed_cancel_reqs earlier to avoid a race
- get rid of POLL_TO_PTR magic
Changes since v8:
- make delayed cancellation conditional again
- add a cancel_kiocb file operation to split delayed vs normal cancel
Changes since v7:
- make delayed cancellation safe and unconditional
Changes since v6:
- reworked cancellation
Changes since v5:
- small changelog updates
- rebased on top of the aio-fsync changes
Changes since v4:
- rebased ontop of Linux 4.16-rc4
Changes since v3:
- remove the pre-sleep ->poll_mask call in vfs_poll,
allow ->get_poll_head to return POLL* values.
Changes since v2:
- removed a double initialization
- new vfs_get_poll_head helper
- document that ->get_poll_head can return NULL
- call ->poll_mask before sleeping
- various ACKs
- add conversion of random to ->poll_mask
- add conversion of af_alg to ->poll_mask
- lacking ->poll_mask support now returns -EINVAL for IOCB_CMD_POLL
- reshuffled the series so that prep patches and everything not
requiring the new in-kernel poll API is in the beginning
Changes since v1:
- handle the NULL ->poll case in vfs_poll
- dropped the file argument to the ->poll_mask socket operation
- replace the ->pre_poll socket operation with ->get_poll_head as
in the file operations
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* [PATCH 01/31] fs: unexport poll_schedule_timeout
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
No users outside of select.c.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/select.c | 3 +--
include/linux/poll.h | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index ba879c51288f..a87f396f0313 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -233,7 +233,7 @@ static void __pollwait(struct file *filp, wait_queue_head_t *wait_address,
add_wait_queue(wait_address, &entry->wait);
}
-int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
+static int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
ktime_t *expires, unsigned long slack)
{
int rc = -EINTR;
@@ -258,7 +258,6 @@ int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
return rc;
}
-EXPORT_SYMBOL(poll_schedule_timeout);
/**
* poll_select_set_timeout - helper function to setup the timeout value
diff --git a/include/linux/poll.h b/include/linux/poll.h
index f45ebd017eaa..a3576da63377 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -96,8 +96,6 @@ struct poll_wqueues {
extern void poll_initwait(struct poll_wqueues *pwq);
extern void poll_freewait(struct poll_wqueues *pwq);
-extern int poll_schedule_timeout(struct poll_wqueues *pwq, int state,
- ktime_t *expires, unsigned long slack);
extern u64 select_estimate_accuracy(struct timespec64 *tv);
#define MAX_INT64_SECONDS (((s64)(~((u64)0)>>1)/HZ)-1)
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 02/31] fs: cleanup do_pollfd
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
Use straightline code with failure handling gotos instead of a lot
of nested conditionals.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/select.c | 48 +++++++++++++++++++++++-------------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index a87f396f0313..25da26253485 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -812,34 +812,32 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
bool *can_busy_poll,
__poll_t busy_flag)
{
- __poll_t mask;
- int fd;
-
- mask = 0;
- fd = pollfd->fd;
- if (fd >= 0) {
- struct fd f = fdget(fd);
- mask = EPOLLNVAL;
- if (f.file) {
- /* userland u16 ->events contains POLL... bitmap */
- __poll_t filter = demangle_poll(pollfd->events) |
- EPOLLERR | EPOLLHUP;
- mask = DEFAULT_POLLMASK;
- if (f.file->f_op->poll) {
- pwait->_key = filter;
- pwait->_key |= busy_flag;
- mask = f.file->f_op->poll(f.file, pwait);
- if (mask & busy_flag)
- *can_busy_poll = true;
- }
- /* Mask out unneeded events. */
- mask &= filter;
- fdput(f);
- }
+ int fd = pollfd->fd;
+ __poll_t mask = 0, filter;
+ struct fd f;
+
+ if (fd < 0)
+ goto out;
+ mask = EPOLLNVAL;
+ f = fdget(fd);
+ if (!f.file)
+ goto out;
+
+ /* userland u16 ->events contains POLL... bitmap */
+ filter = demangle_poll(pollfd->events) | EPOLLERR | EPOLLHUP;
+ mask = DEFAULT_POLLMASK;
+ if (f.file->f_op->poll) {
+ pwait->_key = filter | busy_flag;
+ mask = f.file->f_op->poll(f.file, pwait);
+ if (mask & busy_flag)
+ *can_busy_poll = true;
}
+ mask &= filter; /* Mask out unneeded events. */
+ fdput(f);
+
+out:
/* ... and so does ->revents */
pollfd->revents = mangle_poll(mask);
-
return mask;
}
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 03/31] fs: update documentation to mention __poll_t and match the code
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/filesystems/Locking | 2 +-
Documentation/filesystems/vfs.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 75d2d57e2c44..220bba28f72b 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -439,7 +439,7 @@ prototypes:
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
- unsigned int (*poll) (struct file *, struct poll_table_struct *);
+ __poll_t (*poll) (struct file *, struct poll_table_struct *);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 5fd325df59e2..f608180ad59d 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -856,7 +856,7 @@ struct file_operations {
ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
- unsigned int (*poll) (struct file *, struct poll_table_struct *);
+ __poll_t (*poll) (struct file *, struct poll_table_struct *);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 04/31] fs: add new vfs_poll and file_can_poll helpers
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
These abstract out calls to the poll method in preparation for changes
in how we poll.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
drivers/staging/comedi/drivers/serial2002.c | 4 ++--
drivers/vfio/virqfd.c | 2 +-
drivers/vhost/vhost.c | 2 +-
fs/eventpoll.c | 5 ++---
fs/select.c | 23 +++++++--------------
include/linux/poll.h | 12 +++++++++++
mm/memcontrol.c | 2 +-
net/9p/trans_fd.c | 18 ++++------------
virt/kvm/eventfd.c | 2 +-
9 files changed, 32 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/comedi/drivers/serial2002.c b/drivers/staging/comedi/drivers/serial2002.c
index b3f3b4a201af..5471b2212a62 100644
--- a/drivers/staging/comedi/drivers/serial2002.c
+++ b/drivers/staging/comedi/drivers/serial2002.c
@@ -113,7 +113,7 @@ static void serial2002_tty_read_poll_wait(struct file *f, int timeout)
long elapsed;
__poll_t mask;
- mask = f->f_op->poll(f, &table.pt);
+ mask = vfs_poll(f, &table.pt);
if (mask & (EPOLLRDNORM | EPOLLRDBAND | EPOLLIN |
EPOLLHUP | EPOLLERR)) {
break;
@@ -136,7 +136,7 @@ static int serial2002_tty_read(struct file *f, int timeout)
result = -1;
if (!IS_ERR(f)) {
- if (f->f_op->poll) {
+ if (file_can_poll(f)) {
serial2002_tty_read_poll_wait(f, timeout);
if (kernel_read(f, &ch, 1, &pos) == 1)
diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index 085700f1be10..2a1be859ee71 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -166,7 +166,7 @@ int vfio_virqfd_enable(void *opaque,
init_waitqueue_func_entry(&virqfd->wait, virqfd_wakeup);
init_poll_funcptr(&virqfd->pt, virqfd_ptable_queue_proc);
- events = irqfd.file->f_op->poll(irqfd.file, &virqfd->pt);
+ events = vfs_poll(irqfd.file, &virqfd->pt);
/*
* Check if there was an event already pending on the eventfd
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index f3bd8e941224..f6022881f147 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -208,7 +208,7 @@ int vhost_poll_start(struct vhost_poll *poll, struct file *file)
if (poll->wqh)
return 0;
- mask = file->f_op->poll(file, &poll->table);
+ mask = vfs_poll(file, &poll->table);
if (mask)
vhost_poll_wakeup(&poll->wait, 0, 0, poll_to_key(mask));
if (mask & EPOLLERR) {
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 602ca4285b2e..67db22fe99c5 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -884,8 +884,7 @@ static __poll_t ep_item_poll(const struct epitem *epi, poll_table *pt,
pt->_key = epi->event.events;
if (!is_file_epoll(epi->ffd.file))
- return epi->ffd.file->f_op->poll(epi->ffd.file, pt) &
- epi->event.events;
+ return vfs_poll(epi->ffd.file, pt) & epi->event.events;
ep = epi->ffd.file->private_data;
poll_wait(epi->ffd.file, &ep->poll_wait, pt);
@@ -2025,7 +2024,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
/* The target file descriptor must support poll */
error = -EPERM;
- if (!tf.file->f_op->poll)
+ if (!file_can_poll(tf.file))
goto error_tgt_fput;
/* Check if EPOLLWAKEUP is allowed */
diff --git a/fs/select.c b/fs/select.c
index 25da26253485..e30def680b2e 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -502,14 +502,10 @@ static int do_select(int n, fd_set_bits *fds, struct timespec64 *end_time)
continue;
f = fdget(i);
if (f.file) {
- const struct file_operations *f_op;
- f_op = f.file->f_op;
- mask = DEFAULT_POLLMASK;
- if (f_op->poll) {
- wait_key_set(wait, in, out,
- bit, busy_flag);
- mask = (*f_op->poll)(f.file, wait);
- }
+ wait_key_set(wait, in, out, bit,
+ busy_flag);
+ mask = vfs_poll(f.file, wait);
+
fdput(f);
if ((mask & POLLIN_SET) && (in & bit)) {
res_in |= bit;
@@ -825,13 +821,10 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
/* userland u16 ->events contains POLL... bitmap */
filter = demangle_poll(pollfd->events) | EPOLLERR | EPOLLHUP;
- mask = DEFAULT_POLLMASK;
- if (f.file->f_op->poll) {
- pwait->_key = filter | busy_flag;
- mask = f.file->f_op->poll(f.file, pwait);
- if (mask & busy_flag)
- *can_busy_poll = true;
- }
+ pwait->_key = filter | busy_flag;
+ mask = vfs_poll(f.file, pwait);
+ if (mask & busy_flag)
+ *can_busy_poll = true;
mask &= filter; /* Mask out unneeded events. */
fdput(f);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index a3576da63377..7e0fdcf905d2 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,6 +74,18 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
+static inline bool file_can_poll(struct file *file)
+{
+ return file->f_op->poll;
+}
+
+static inline __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+{
+ if (unlikely(!file->f_op->poll))
+ return DEFAULT_POLLMASK;
+ return file->f_op->poll(file, pt);
+}
+
struct poll_table_entry {
struct file *filp;
__poll_t key;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2bd3df3d101a..1695f38630f1 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3849,7 +3849,7 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
if (ret)
goto out_put_css;
- efile.file->f_op->poll(efile.file, &event->pt);
+ vfs_poll(efile.file, &event->pt);
spin_lock(&memcg->event_list_lock);
list_add(&event->list, &memcg->event_list);
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 0cfba919d167..3811775692d0 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -231,7 +231,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
static __poll_t
p9_fd_poll(struct p9_client *client, struct poll_table_struct *pt, int *err)
{
- __poll_t ret, n;
+ __poll_t ret;
struct p9_trans_fd *ts = NULL;
if (client && client->status == Connected)
@@ -243,19 +243,9 @@ p9_fd_poll(struct p9_client *client, struct poll_table_struct *pt, int *err)
return EPOLLERR;
}
- if (!ts->rd->f_op->poll)
- ret = DEFAULT_POLLMASK;
- else
- ret = ts->rd->f_op->poll(ts->rd, pt);
-
- if (ts->rd != ts->wr) {
- if (!ts->wr->f_op->poll)
- n = DEFAULT_POLLMASK;
- else
- n = ts->wr->f_op->poll(ts->wr, pt);
- ret = (ret & ~EPOLLOUT) | (n & ~EPOLLIN);
- }
-
+ ret = vfs_poll(ts->rd, pt);
+ if (ts->rd != ts->wr)
+ ret = (ret & ~EPOLLOUT) | (vfs_poll(ts->wr, pt) & ~EPOLLIN);
return ret;
}
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 6e865e8b5b10..90d30fbe95ae 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -397,7 +397,7 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
* Check if there was an event already pending on the eventfd
* before we registered, and trigger it as if we didn't miss it.
*/
- events = f.file->f_op->poll(f.file, &irqfd->pt);
+ events = vfs_poll(f.file, &irqfd->pt);
if (events & EPOLLIN)
schedule_work(&irqfd->inject);
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 05/31] fs: introduce new ->get_poll_head and ->poll_mask methods
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
->get_poll_head returns the waitqueue that the poll operation is going
to sleep on. Note that this means we can only use a single waitqueue
for the poll, unlike some current drivers that use two waitqueues for
different events. But now that we have keyed wakeups and heavily use
those for poll there aren't that many good reason left to keep the
multiple waitqueues, and if there are any ->poll is still around, the
driver just won't support aio poll.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
Documentation/filesystems/Locking | 7 ++++++-
Documentation/filesystems/vfs.txt | 13 +++++++++++++
fs/select.c | 23 +++++++++++++++++++++++
include/linux/fs.h | 2 ++
include/linux/poll.h | 12 ++++++------
5 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 220bba28f72b..6d227f9d7bd9 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -440,6 +440,8 @@ prototypes:
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
@@ -470,7 +472,7 @@ prototypes:
};
locking rules:
- All may block.
+ All except for ->poll_mask may block.
->llseek() locking has moved from llseek to the individual llseek
implementations. If your fs is not using generic_file_llseek, you
@@ -498,6 +500,9 @@ in sys_read() and friends.
the lease within the individual filesystem to record the result of the
operation
+->poll_mask can be called with or without the waitqueue lock for the waitqueue
+returned from ->get_poll_head.
+
--------------------------- dquot_operations -------------------------------
prototypes:
int (*write_dquot) (struct dquot *);
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index f608180ad59d..829a7b7857a4 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -857,6 +857,8 @@ struct file_operations {
ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
int (*iterate) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
@@ -901,6 +903,17 @@ otherwise noted.
activity on this file and (optionally) go to sleep until there
is activity. Called by the select(2) and poll(2) system calls
+ get_poll_head: Returns the struct wait_queue_head that callers can
+ wait on. Callers need to check the returned events using ->poll_mask
+ once woken. Can return NULL to indicate polling is not supported,
+ or any error code using the ERR_PTR convention to indicate that a
+ grave error occured and ->poll_mask shall not be called.
+
+ poll_mask: return the mask of EPOLL* values describing the file descriptor
+ state. Called either before going to sleep on the waitqueue returned by
+ get_poll_head, or after it has been woken. If ->get_poll_head and
+ ->poll_mask are implemented ->poll does not need to be implement.
+
unlocked_ioctl: called by the ioctl(2) system call.
compat_ioctl: called by the ioctl(2) system call when 32 bit system calls
diff --git a/fs/select.c b/fs/select.c
index e30def680b2e..bc3cc0f98896 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -34,6 +34,29 @@
#include <linux/uaccess.h>
+__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+{
+ if (file->f_op->poll) {
+ return file->f_op->poll(file, pt);
+ } else if (file_has_poll_mask(file)) {
+ unsigned int events = poll_requested_events(pt);
+ struct wait_queue_head *head;
+
+ if (pt && pt->_qproc) {
+ head = file->f_op->get_poll_head(file, events);
+ if (!head)
+ return DEFAULT_POLLMASK;
+ if (IS_ERR(head))
+ return EPOLLERR;
+ pt->_qproc(file, head, pt);
+ }
+
+ return file->f_op->poll_mask(file, events);
+ } else {
+ return DEFAULT_POLLMASK;
+ }
+}
+EXPORT_SYMBOL_GPL(vfs_poll);
/*
* Estimate expected accuracy in ns from a timeval.
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..4b6045ebb2f2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1711,6 +1711,8 @@ struct file_operations {
int (*iterate) (struct file *, struct dir_context *);
int (*iterate_shared) (struct file *, struct dir_context *);
__poll_t (*poll) (struct file *, struct poll_table_struct *);
+ struct wait_queue_head * (*get_poll_head)(struct file *, __poll_t);
+ __poll_t (*poll_mask) (struct file *, __poll_t);
long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
int (*mmap) (struct file *, struct vm_area_struct *);
diff --git a/include/linux/poll.h b/include/linux/poll.h
index 7e0fdcf905d2..fdf86b4cbc71 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -74,18 +74,18 @@ static inline void init_poll_funcptr(poll_table *pt, poll_queue_proc qproc)
pt->_key = ~(__poll_t)0; /* all events enabled */
}
-static inline bool file_can_poll(struct file *file)
+static inline bool file_has_poll_mask(struct file *file)
{
- return file->f_op->poll;
+ return file->f_op->get_poll_head && file->f_op->poll_mask;
}
-static inline __poll_t vfs_poll(struct file *file, struct poll_table_struct *pt)
+static inline bool file_can_poll(struct file *file)
{
- if (unlikely(!file->f_op->poll))
- return DEFAULT_POLLMASK;
- return file->f_op->poll(file, pt);
+ return file->f_op->poll || file_has_poll_mask(file);
}
+__poll_t vfs_poll(struct file *file, struct poll_table_struct *pt);
+
struct poll_table_entry {
struct file *filp;
__poll_t key;
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 06/31] aio: simplify KIOCB_KEY handling
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
No need to pass the key field to lookup_iocb to compare it with KIOCB_KEY,
as we can do that right after retrieving it from userspace. Also move the
KIOCB_KEY definition to aio.c as it is an internal value not used by any
other place in the kernel.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 14 +++++++-------
include/linux/aio.h | 2 --
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index f3eae5d5771b..2e0bc04f7920 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -46,6 +46,8 @@
#include "internal.h"
+#define KIOCB_KEY 0
+
#define AIO_RING_MAGIC 0xa10a10a1
#define AIO_RING_COMPAT_FEATURES 1
#define AIO_RING_INCOMPAT_FEATURES 0
@@ -1812,15 +1814,12 @@ COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
* Finds a given iocb for cancellation.
*/
static struct aio_kiocb *
-lookup_kiocb(struct kioctx *ctx, struct iocb __user *iocb, u32 key)
+lookup_kiocb(struct kioctx *ctx, struct iocb __user *iocb)
{
struct aio_kiocb *kiocb;
assert_spin_locked(&ctx->ctx_lock);
- if (key != KIOCB_KEY)
- return NULL;
-
/* TODO: use a hash or array, this sucks. */
list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
if (kiocb->ki_user_iocb == iocb)
@@ -1847,9 +1846,10 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
u32 key;
int ret;
- ret = get_user(key, &iocb->aio_key);
- if (unlikely(ret))
+ if (unlikely(get_user(key, &iocb->aio_key)))
return -EFAULT;
+ if (unlikely(key != KIOCB_KEY))
+ return -EINVAL;
ctx = lookup_ioctx(ctx_id);
if (unlikely(!ctx))
@@ -1857,7 +1857,7 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
spin_lock_irq(&ctx->ctx_lock);
- kiocb = lookup_kiocb(ctx, iocb, key);
+ kiocb = lookup_kiocb(ctx, iocb);
if (kiocb)
ret = kiocb_cancel(kiocb);
else
diff --git a/include/linux/aio.h b/include/linux/aio.h
index 9d8aabecfe2d..b83e68dd006f 100644
--- a/include/linux/aio.h
+++ b/include/linux/aio.h
@@ -8,8 +8,6 @@ struct kioctx;
struct kiocb;
struct mm_struct;
-#define KIOCB_KEY 0
-
typedef int (kiocb_cancel_fn)(struct kiocb *);
/* prototypes */
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 07/31] aio: simplify cancellation
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
With the current aio code there is no need for the magic KIOCB_CANCELLED
value, as a cancelation just kicks the driver to queue the completion
ASAP, with all actual completion handling done in another thread. Given
that both the completion path and cancelation take the context lock there
is no need for magic cmpxchg loops either. If we remove iocbs from the
active list before calling ->ki_cancel we can also rely on the invariant
thay anything found on the list has a ->ki_cancel callback and can be
cancelled, further simplifing the code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 49 ++++++-------------------------------------------
1 file changed, 6 insertions(+), 43 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 2e0bc04f7920..8991baa38d5d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -164,19 +164,6 @@ struct fsync_iocb {
bool datasync;
};
-/*
- * We use ki_cancel == KIOCB_CANCELLED to indicate that a kiocb has been either
- * cancelled or completed (this makes a certain amount of sense because
- * successful cancellation - io_cancel() - does deliver the completion to
- * userspace).
- *
- * And since most things don't implement kiocb cancellation and we'd really like
- * kiocb completion to be lockless when possible, we use ki_cancel to
- * synchronize cancellation and completion - we only set it to KIOCB_CANCELLED
- * with xchg() or cmpxchg(), see batch_complete_aio() and kiocb_cancel().
- */
-#define KIOCB_CANCELLED ((void *) (~0ULL))
-
struct aio_kiocb {
union {
struct kiocb rw;
@@ -574,27 +561,6 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
}
EXPORT_SYMBOL(kiocb_set_cancel_fn);
-static int kiocb_cancel(struct aio_kiocb *kiocb)
-{
- kiocb_cancel_fn *old, *cancel;
-
- /*
- * Don't want to set kiocb->ki_cancel = KIOCB_CANCELLED unless it
- * actually has a cancel function, hence the cmpxchg()
- */
-
- cancel = READ_ONCE(kiocb->ki_cancel);
- do {
- if (!cancel || cancel == KIOCB_CANCELLED)
- return -EINVAL;
-
- old = cancel;
- cancel = cmpxchg(&kiocb->ki_cancel, old, KIOCB_CANCELLED);
- } while (cancel != old);
-
- return cancel(&kiocb->rw);
-}
-
/*
* free_ioctx() should be RCU delayed to synchronize against the RCU
* protected lookup_ioctx() and also needs process context to call
@@ -641,9 +607,8 @@ static void free_ioctx_users(struct percpu_ref *ref)
while (!list_empty(&ctx->active_reqs)) {
req = list_first_entry(&ctx->active_reqs,
struct aio_kiocb, ki_list);
-
list_del_init(&req->ki_list);
- kiocb_cancel(req);
+ req->ki_cancel(&req->rw);
}
spin_unlock_irq(&ctx->ctx_lock);
@@ -1843,8 +1808,8 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
{
struct kioctx *ctx;
struct aio_kiocb *kiocb;
+ int ret = -EINVAL;
u32 key;
- int ret;
if (unlikely(get_user(key, &iocb->aio_key)))
return -EFAULT;
@@ -1856,13 +1821,11 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
return -EINVAL;
spin_lock_irq(&ctx->ctx_lock);
-
kiocb = lookup_kiocb(ctx, iocb);
- if (kiocb)
- ret = kiocb_cancel(kiocb);
- else
- ret = -EINVAL;
-
+ if (kiocb) {
+ list_del_init(&kiocb->ki_list);
+ ret = kiocb->ki_cancel(&kiocb->rw);
+ }
spin_unlock_irq(&ctx->ctx_lock);
if (!ret) {
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 08/31] aio: implement IOCB_CMD_POLL
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
Simple one-shot poll through the io_submit() interface. To poll for
a file descriptor the application should submit an iocb of type
IOCB_CMD_POLL. It will poll the fd for the events specified in the
the first 32 bits of the aio_buf field of the iocb.
Unlike poll or epoll without EPOLLONESHOT this interface always works
in one shot mode, that is once the iocb is completed, it will have to be
resubmitted.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/aio.c | 134 ++++++++++++++++++++++++++++++++++-
include/uapi/linux/aio_abi.h | 6 +-
2 files changed, 135 insertions(+), 5 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..4d1eabce6659 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -5,6 +5,7 @@
* Implements an efficient asynchronous io interface.
*
* Copyright 2000, 2001, 2002 Red Hat, Inc. All Rights Reserved.
+ * Copyright 2018 Christoph Hellwig.
*
* See ../COPYING for licensing terms.
*/
@@ -164,10 +165,22 @@ struct fsync_iocb {
bool datasync;
};
+struct poll_iocb {
+ struct file *file;
+ __poll_t events;
+ struct wait_queue_head *head;
+
+ union {
+ struct wait_queue_entry wait;
+ struct work_struct work;
+ };
+};
+
struct aio_kiocb {
union {
struct kiocb rw;
struct fsync_iocb fsync;
+ struct poll_iocb poll;
};
struct kioctx *ki_ctx;
@@ -1558,7 +1571,6 @@ static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
if (unlikely(iocb->aio_buf || iocb->aio_offset || iocb->aio_nbytes ||
iocb->aio_rw_flags))
return -EINVAL;
-
req->file = fget(iocb->aio_fildes);
if (unlikely(!req->file))
return -EBADF;
@@ -1573,6 +1585,124 @@ static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
return -EIOCBQUEUED;
}
+/* need to use list_del_init so we can check if item was present */
+static inline bool __aio_poll_remove(struct poll_iocb *req)
+{
+ if (list_empty(&req->wait.entry))
+ return false;
+ list_del_init(&req->wait.entry);
+ return true;
+}
+
+static inline void __aio_poll_complete(struct poll_iocb *req, __poll_t mask)
+{
+ struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, poll);
+
+ fput(req->file);
+ aio_complete(iocb, mangle_poll(mask), 0);
+}
+
+static void aio_poll_work(struct work_struct *work)
+{
+ struct poll_iocb *req = container_of(work, struct poll_iocb, work);
+
+ __aio_poll_complete(req, req->events);
+}
+
+static int aio_poll_cancel(struct kiocb *iocb)
+{
+ struct aio_kiocb *aiocb = container_of(iocb, struct aio_kiocb, rw);
+ struct poll_iocb *req = &aiocb->poll;
+ struct wait_queue_head *head = req->head;
+ bool found = false;
+
+ spin_lock(&head->lock);
+ found = __aio_poll_remove(req);
+ spin_unlock(&head->lock);
+
+ if (found) {
+ req->events = 0;
+ INIT_WORK(&req->work, aio_poll_work);
+ schedule_work(&req->work);
+ }
+ return 0;
+}
+
+static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
+ void *key)
+{
+ struct poll_iocb *req = container_of(wait, struct poll_iocb, wait);
+ struct file *file = req->file;
+ __poll_t mask = key_to_poll(key);
+
+ assert_spin_locked(&req->head->lock);
+
+ /* for instances that support it check for an event match first: */
+ if (mask && !(mask & req->events))
+ return 0;
+
+ mask = file->f_op->poll_mask(file, req->events);
+ if (!mask)
+ return 0;
+
+ __aio_poll_remove(req);
+
+ req->events = mask;
+ INIT_WORK(&req->work, aio_poll_work);
+ schedule_work(&req->work);
+ return 1;
+}
+
+static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb)
+{
+ struct kioctx *ctx = aiocb->ki_ctx;
+ struct poll_iocb *req = &aiocb->poll;
+ __poll_t mask;
+
+ /* reject any unknown events outside the normal event mask. */
+ if ((u16)iocb->aio_buf != iocb->aio_buf)
+ return -EINVAL;
+ /* reject fields that are not defined for poll */
+ if (iocb->aio_offset || iocb->aio_nbytes || iocb->aio_rw_flags)
+ return -EINVAL;
+
+ req->events = demangle_poll(iocb->aio_buf) | POLLERR | POLLHUP;
+ req->file = fget(iocb->aio_fildes);
+ if (unlikely(!req->file))
+ return -EBADF;
+ if (!file_has_poll_mask(req->file))
+ goto out_fail;
+
+ req->head = req->file->f_op->get_poll_head(req->file, req->events);
+ if (!req->head)
+ goto out_fail;
+ if (IS_ERR(req->head)) {
+ mask = EPOLLERR;
+ goto done;
+ }
+
+ init_waitqueue_func_entry(&req->wait, aio_poll_wake);
+ aiocb->ki_cancel = aio_poll_cancel;
+
+ spin_lock_irq(&ctx->ctx_lock);
+ list_add_tail(&aiocb->ki_list, &ctx->active_reqs);
+
+ spin_lock(&req->head->lock);
+ mask = req->file->f_op->poll_mask(req->file, req->events);
+ if (!mask)
+ __add_wait_queue(req->head, &req->wait);
+ spin_unlock(&req->head->lock);
+
+ spin_unlock_irq(&ctx->ctx_lock);
+done:
+ if (mask)
+ __aio_poll_complete(req, mask);
+ return -EIOCBQUEUED;
+out_fail:
+ fput(req->file);
+ return -EINVAL; /* same as no support for IOCB_CMD_POLL */
+}
+
static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
struct iocb *iocb, bool compat)
{
@@ -1641,6 +1771,8 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
break;
case IOCB_CMD_FDSYNC:
ret = aio_fsync(&req->fsync, iocb, true);
+ case IOCB_CMD_POLL:
+ ret = aio_poll(req, iocb);
break;
default:
pr_debug("invalid aio operation %d\n", iocb->aio_lio_opcode);
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index 2c0a3415beee..ed0185945bb2 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -39,10 +39,8 @@ enum {
IOCB_CMD_PWRITE = 1,
IOCB_CMD_FSYNC = 2,
IOCB_CMD_FDSYNC = 3,
- /* These two are experimental.
- * IOCB_CMD_PREADX = 4,
- * IOCB_CMD_POLL = 5,
- */
+ /* 4 was the experimental IOCB_CMD_PREADX */
+ IOCB_CMD_POLL = 5,
IOCB_CMD_NOOP = 6,
IOCB_CMD_PREADV = 7,
IOCB_CMD_PWRITEV = 8,
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* [PATCH 09/31] aio: try to complete poll iocbs without context switch
From: Christoph Hellwig @ 2018-05-22 11:30 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180522113108.25713-1-hch@lst.de>
If we can acquire ctx_lock without spinning we can just remove our
iocb from the active_reqs list, and thus complete the iocbs from the
wakeup context.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 4d1eabce6659..85d9102431db 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1632,6 +1632,7 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
void *key)
{
struct poll_iocb *req = container_of(wait, struct poll_iocb, wait);
+ struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, poll);
struct file *file = req->file;
__poll_t mask = key_to_poll(key);
@@ -1647,9 +1648,22 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
__aio_poll_remove(req);
- req->events = mask;
- INIT_WORK(&req->work, aio_poll_work);
- schedule_work(&req->work);
+ /*
+ * Try completing without a context switch if we can acquire ctx_lock
+ * without spinning. Otherwise we need to defer to a workqueue to
+ * avoid a deadlock due to the lock order.
+ */
+ if (spin_trylock(&iocb->ki_ctx->ctx_lock)) {
+ list_del_init(&iocb->ki_list);
+ spin_unlock(&iocb->ki_ctx->ctx_lock);
+
+ __aio_poll_complete(req, mask);
+ } else {
+ req->events = mask;
+ INIT_WORK(&req->work, aio_poll_work);
+ schedule_work(&req->work);
+ }
+
return 1;
}
--
2.17.0
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox