From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
To: Tang Chen <tangchen@cn.fujitsu.com>, <bcrl@kvack.org>
Cc: <viro@zeniv.linux.org.uk>, <jmoyer@redhat.com>,
<kosaki.motohiro@gmail.com>, <kosaki.motohiro@jp.fujitsu.com>,
<guz.fnst@cn.fujitsu.com>, <linux-fsdevel@vger.kernel.org>,
<linux-aio@kvack.org>, <linux-kernel@vger.kernel.org>,
<miaox@cn.fujitsu.com>
Subject: [Update v2 PATCH 2/2] aio, mem-hotplug: Add memory barrier to aio ring page migration.
Date: Wed, 5 Mar 2014 16:17:42 +0900 [thread overview]
Message-ID: <5316CF96.20902@jp.fujitsu.com> (raw)
In-Reply-To: <530F3327.8020205@jp.fujitsu.com>
When doing aio ring page migration, we migrated the page, and update
ctx->ring_pages[]. Like the following:
aio_migratepage()
|-> migrate_page_copy(new, old)
| ...... /* Need barrier here */
|-> ctx->ring_pages[idx] = new
Actually, we need a memory barrier between these two operations.
Otherwise, if ctx->ring_pages[] is updated before memory copy due to
the compiler optimization, other processes may have an opportunity
to access to the not fully initialized new ring page.
So add a wmb and rmb to synchronize them.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
---
v2: change smp_rmb() to smp_read_barrier_depends(). Thanks Miao.
---
fs/aio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c
index 50c089c..98c7f2d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -327,6 +327,14 @@ static int aio_migratepage(struct address_space *mapping, struct page *new,
pgoff_t idx;
spin_lock_irqsave(&ctx->completion_lock, flags);
migrate_page_copy(new, old);
+
+ /*
+ * Ensure memory copy is finished before updating
+ * ctx->ring_pages[]. Otherwise other processes may access to
+ * new ring pages which are not fully initialized.
+ */
+ smp_wmb();
+
idx = old->index;
if (idx < (pgoff_t)ctx->nr_pages) {
/* And only do the move if things haven't changed */
@@ -1074,6 +1082,12 @@ static long aio_read_events_ring(struct kioctx *ctx,
page = ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE];
pos %= AIO_EVENTS_PER_PAGE;
+ /*
+ * Ensure that the page's data was copied from old one by
+ * aio_migratepage().
+ */
+ smp_read_barrier_depends();
+
ev = kmap(page);
copy_ret = copy_to_user(event + ret, ev + pos,
sizeof(*ev) * avail);
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
To: Tang Chen <tangchen@cn.fujitsu.com>, <bcrl@kvack.org>
Cc: <viro@zeniv.linux.org.uk>, <jmoyer@redhat.com>,
<kosaki.motohiro@gmail.com>, <kosaki.motohiro@jp.fujitsu.com>,
<guz.fnst@cn.fujitsu.com>, <linux-fsdevel@vger.kernel.org>,
<linux-aio@kvack.org>, <linux-kernel@vger.kernel.org>,
<miaox@cn.fujitsu.com>
Subject: [Update v2 PATCH 2/2] aio, mem-hotplug: Add memory barrier to aio ring page migration.
Date: Wed, 5 Mar 2014 16:17:42 +0900 [thread overview]
Message-ID: <5316CF96.20902@jp.fujitsu.com> (raw)
In-Reply-To: <530F3327.8020205@jp.fujitsu.com>
When doing aio ring page migration, we migrated the page, and update
ctx->ring_pages[]. Like the following:
aio_migratepage()
|-> migrate_page_copy(new, old)
| ...... /* Need barrier here */
|-> ctx->ring_pages[idx] = new
Actually, we need a memory barrier between these two operations.
Otherwise, if ctx->ring_pages[] is updated before memory copy due to
the compiler optimization, other processes may have an opportunity
to access to the not fully initialized new ring page.
So add a wmb and rmb to synchronize them.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
---
v2: change smp_rmb() to smp_read_barrier_depends(). Thanks Miao.
---
fs/aio.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c
index 50c089c..98c7f2d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -327,6 +327,14 @@ static int aio_migratepage(struct address_space *mapping, struct page *new,
pgoff_t idx;
spin_lock_irqsave(&ctx->completion_lock, flags);
migrate_page_copy(new, old);
+
+ /*
+ * Ensure memory copy is finished before updating
+ * ctx->ring_pages[]. Otherwise other processes may access to
+ * new ring pages which are not fully initialized.
+ */
+ smp_wmb();
+
idx = old->index;
if (idx < (pgoff_t)ctx->nr_pages) {
/* And only do the move if things haven't changed */
@@ -1074,6 +1082,12 @@ static long aio_read_events_ring(struct kioctx *ctx,
page = ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE];
pos %= AIO_EVENTS_PER_PAGE;
+ /*
+ * Ensure that the page's data was copied from old one by
+ * aio_migratepage().
+ */
+ smp_read_barrier_depends();
+
ev = kmap(page);
copy_ret = copy_to_user(event + ret, ev + pos,
sizeof(*ev) * avail);
next prev parent reply other threads:[~2014-03-05 7:17 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-27 10:40 [PATCH 0/2] Bug fix in aio ring page migration Tang Chen
2014-02-27 10:40 ` Tang Chen
2014-02-27 10:40 ` [PATCH 1/2] aio, memory-hotplug: Fix confliction when migrating and accessing ring pages Tang Chen
2014-02-27 10:40 ` Tang Chen
2014-03-05 19:23 ` Jeff Moyer
2014-03-05 19:23 ` Jeff Moyer
2014-02-27 10:40 ` [PATCH 2/2] aio, mem-hotplug: Add memory barrier to aio ring page migration Tang Chen
2014-02-27 10:40 ` Tang Chen
2014-02-27 12:06 ` Yasuaki Ishimatsu
2014-02-27 12:06 ` Yasuaki Ishimatsu
2014-02-27 12:44 ` [Update PATCH " Yasuaki Ishimatsu
2014-02-27 12:44 ` Yasuaki Ishimatsu
2014-03-04 5:35 ` Miao Xie
2014-03-04 5:35 ` Miao Xie
2014-03-05 3:04 ` KOSAKI Motohiro
2014-03-05 3:04 ` KOSAKI Motohiro
2014-03-05 6:59 ` Yasuaki Ishimatsu
2014-03-05 6:59 ` Yasuaki Ishimatsu
2014-03-05 7:17 ` Yasuaki Ishimatsu [this message]
2014-03-05 7:17 ` [Update v2 " Yasuaki Ishimatsu
2014-02-27 14:57 ` [PATCH " Benjamin LaHaise
2014-02-27 14:57 ` Benjamin LaHaise
2014-02-28 1:29 ` Gu Zheng
2014-02-28 7:25 ` Yasuaki Ishimatsu
2014-02-28 7:25 ` Yasuaki Ishimatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5316CF96.20902@jp.fujitsu.com \
--to=isimatu.yasuaki@jp.fujitsu.com \
--cc=bcrl@kvack.org \
--cc=guz.fnst@cn.fujitsu.com \
--cc=jmoyer@redhat.com \
--cc=kosaki.motohiro@gmail.com \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-aio@kvack.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miaox@cn.fujitsu.com \
--cc=tangchen@cn.fujitsu.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.