lkml.org 
[lkml]   [2014]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESEND v2 PATCH 1/2] aio, memory-hotplug: Fix confliction when migrating and accessing ring pages.
Hi Ben,
Sorry for late.
On 03/14/2014 11:14 PM, Benjamin LaHaise wrote:

> Hi Gu,
>
> On Fri, Mar 14, 2014 at 06:25:16PM +0800, Gu Zheng wrote:
>> Hi Ben,
>> On 03/13/2014 06:17 AM, Benjamin LaHaise wrote:
>>
>>> Hello Tang,
>>>
>>> On Wed, Mar 12, 2014 at 01:25:26PM +0800, Tang Chen wrote:
>>> ... <snip> ...
>>>
>>>>> Another spot is in
>>>>> aio_read_events_ring() where head and tail are fetched from the ring
>>>>> without
>>>>> any locking. I also fear we'll be introducing new performance issues with
>>>>> all the additonal spinlock bouncing, despite the fact that is only ever
>>>>> needed for migration. I'm going to continue looking into this today and
>>>>> will try to send out a followup to this email later.
>>>>
>>>> In the beginning of aio_read_events_ring(), it reads head and tail, not
>>>> write.
>>>> So even if ring pages are migrated, the contents of the pages will not
>>>> be changed.
>>>> So reading it is OK, from old page or from the new page, I think.
>>>
>>> Your assumption that reading it is okay is incorrect. Since we do not have
>>> a reference on the page at that point, it is possible that the read of the
>>> page takes place after the page has been freed and allocated to another part
>>> of the kernel. This would result in the read returning invalid information.
>>
>> What about the following patch? It adds additional reference to protect the page
>> avoid being freed when we reading it.
>> ps.It is applied on linux-next(3-13).
>
> I think that's even worse than the spinlock approach since we'll end up
> bouncing around the struct page's cacheline in addition to spinlock we're
> going to end up taking anyways.

But we can not use spinlock approach to avoid this issue in aio_read_events_ring(),
because we need to copy events to user space. And on the other side, it will break
the concurrency of aio_read_events_ring() and aio_complete().
Besides, IMHO, the problem you mentioned above is almost insignificant when reading
events.
Any better solution? Other guys?

Regards,
Gu

>
> -ben




\
 
 \ /
  Last update: 2014-03-16 04:01    [W:0.049 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site