linux-kernel - Re: [PATCH 2/2] aio: make aio_read_events_ring be aware of aio

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 03 Mar 2014 10:17:40 +0800
From:	Gu Zheng <guz.fnst@...fujitsu.com>
To:	Kent Overstreet <kmo@...erainc.com>
CC:	Benjamin <bcrl@...ck.org>, Jens <axboe@...nel.dk>,
	linux-aio@...ck.org, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] aio: make aio_read_events_ring be aware of aio_complete

Hi Kent,
Sorry for late replay.
On 03/01/2014 04:52 AM, Kent Overstreet wrote:

> On Fri, Feb 28, 2014 at 06:27:18PM +0800, Gu Zheng wrote:
>> Commit 5ffac122dbda8(aio: Don't use ctx->tail unnecessarily) uses
>> ring->tail rather than the ctx->tail, but with this change, we fetch 'tail'
>> only once at the start, so that we can not be aware of adding event by aio_complete
>> when reading events. It seems a regression.
> 
> "Seems a regression" is not good enough. What breaks? 

Previously without the commit 5ffac122dbda8, we used tail from aio context to judge
whether the ring buffer has events(have not been read) each time, as following:
avail = (head <= ctx->tail ? ctx->tail : ctx->nr_events) - head;
so that we can be aware of new events that added by aio_complete when we are reading the
current ones, and we can read the new added events together.
E.g. we want to read 10 events, only 8 events now, add 2 events added while we copying
current 8 events to userspace, and we can read the 2 new events in the next circle.
But with the commit 5ffac122dbda8, we fetch tail from ring buffer only once when we start
to read events, so that we can not be aware of events that added by aio_complete while we
are reading current events(e.g. copy currents events to user space). Such as the case
mentioned above can only read 8 events, though these are 2 new events in the ring buffer.
Is not it a regression, or am I missing something?

Regards,
Gu

> 
>> So here we fetch the ring->tail in start of the loop each time to make it be
>> aware of adding event from aio_complete.
>>
>> Signed-off-by: Gu Zheng <guz.fnst@...fujitsu.com>
>> ---
>>  fs/aio.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index 7eaa631..f5b8551 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1029,10 +1029,14 @@ static long aio_read_events_ring(struct kioctx *ctx,
>>  		struct io_event *ev;
>>  		struct page *page;
>>  
>> -		avail = (head <= tail ?  tail : ctx->nr_events) - head;
>> +		ring = kmap_atomic(ctx->ring_pages[0]);
>> +		tail = ring->tail;
>> +		kunmap_atomic(ring);
>> +
>>  		if (head == tail)
>>  			break;
>>  
>> +		avail = (head <= tail ?  tail : ctx->nr_events) - head;
>>  		avail = min(avail, nr - ret);
>>  		avail = min_t(long, avail, AIO_EVENTS_PER_PAGE -
>>  			    ((head + AIO_EVENTS_OFFSET) % AIO_EVENTS_PER_PAGE));
>> -- 
>> 1.7.7
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/