From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-pg1-f195.google.com ([209.85.215.195]:42451 "EHLO
        mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S2407260AbeKWHm5 (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Fri, 23 Nov 2018 02:42:57 -0500
Received: by mail-pg1-f195.google.com with SMTP id d72so2143223pga.9
        for <linux-fsdevel@vger.kernel.org>; Thu, 22 Nov 2018 13:01:50 -0800 (PST)
Subject: Re: [PATCH 8/8] aio: support for IO polling
To: Jan Kara <jack@suse.cz>
Cc: linux-block@vger.kernel.org, linux-aio@kvack.org,
        linux-fsdevel@vger.kernel.org
References: <20181120171953.1258-1-axboe@kernel.dk>
 <20181120171953.1258-9-axboe@kernel.dk>
 <20181122111351.GD9840@quack2.suse.cz>
From: Jens Axboe <axboe@kernel.dk>
Message-ID: <42b38b07-3120-095e-44ac-bcda88b6412c@kernel.dk>
Date: Thu, 22 Nov 2018 14:01:45 -0700
MIME-Version: 1.0
In-Reply-To: <20181122111351.GD9840@quack2.suse.cz>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On 11/22/18 4:13 AM, Jan Kara wrote:
> 
> On Tue 20-11-18 10:19:53, Jens Axboe wrote:
>> +/*
>> + * We can't just wait for polled events to come to us, we have to actively
>> + * find and complete them.
>> + */
>> +static void aio_iopoll_reap_events(struct kioctx *ctx)
>> +{
>> +	if (!(ctx->flags & IOCTX_FLAG_IOPOLL))
>> +		return;
>> +
>> +	while (!list_empty_careful(&ctx->poll_submitted) ||
>> +	       !list_empty(&ctx->poll_completing)) {
>> +		unsigned int nr_events = 0;
>> +
>> +		__aio_iopoll_check(ctx, NULL, &nr_events, 1, UINT_MAX);
>> +	}
>> +}
>> +
>> +static int aio_iopoll_check(struct kioctx *ctx, long min_nr, long nr,
>> +			    struct io_event __user *event)
>> +{
>> +	unsigned int nr_events = 0;
>> +	int ret = 0;
>> +
>> +	/* * Only allow one thread polling at a time */
>> +	if (test_and_set_bit(0, &ctx->getevents_busy))
>> +		return -EBUSY;
>> +
>> +	while (!nr_events || !need_resched()) {
>> +		int tmin = 0;
>> +
>> +		if (nr_events < min_nr)
>> +			tmin = min_nr - nr_events;
>> +
>> +		ret = __aio_iopoll_check(ctx, event, &nr_events, tmin, nr);
>> +		if (ret <= 0)
>> +			break;
>> +		ret = 0;
>> +	}
>> +
>> +	clear_bit(0, &ctx->getevents_busy);
>> +	return nr_events ? nr_events : ret;
>> +}
> 
> Hum, what if userspace calls io_destroy() while another process is polling
> for events on the same kioctx? It seems we'd be reaping events from two
> processes in parallel in that case which will result in various
> "interesting" effects like ctx->poll_completing list corruption...

I've replaced the ->getevents_busy with a mutex, and we also protect
the ->dead check inside that mutex. That ensures that destroy can't
proceed before a potential caller is inside getevents(), and that
getevents() sees if the ctx is being destroyed.

-- 
Jens Axboe