Messages in this thread | | | Date | Sun, 16 Mar 2014 10:06:05 +0800 | From | Gu Zheng <> | Subject | Re: [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
| |