<div></div><div style="color:#000; font-size: 14px;font-family: arial;"><div>yes, this patch more prefer than I summitted. </div><div><br></div></div><div></div><div id="spnEditorSign" name="100"><div><div style="font-size: 14px;font-family: arial;"><div><em><span style="font-family: comic sans ms; font-size: 10px;">--------------------</span></em></div><div><em><span style="font-family: comic sans ms; font-size: 10px;">黄敏飞平台研发部</span></em></div><div><em><span style="font-family: comic sans ms; font-size: 10px;">上海优刻得信息科技有限公司</span></em></div><div><em><span style="font-family: comic sans ms; font-size: 10px;">Ucloud--做中国最专业的IAAS运营商</span></em></div><div><em><span style="font-family: comic sans ms; font-size: 10px;">QQ: 805852007</span></em></div><div><em><span style="font-family: comic sans ms; font-size: 10px;">地址：上海市杨浦区国定东路200号3号楼2楼</span></em></div></div></div></div><div></div><div><br></div><div></div><div>在2014年06月28 03时29分，"Mike Snitzer"<snitzer@redhat.com>写道：</div><blockquote id="isReplyContent" style="padding-left:1ex; margin: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid"><br> On Fri, Jun 27 2014 at 2:44pm -0400,<br>Mikulas Patocka <<a href="mailto:mpatocka@redhat.com">mpatocka@redhat.com</a>> wrote:<br><br>> <br>> <br>> On Fri, 27 Jun 2014, Joe Thornber wrote:<br>> <br>> > On Fri, Jun 27, 2014 at 12:01:30PM +0800, Minfei Huang wrote:<br>> > > The io address in callback function will become the danging point,<br>> > > cause by the thread of sync io wakes up by other threads<br>> > > and return to relieve the io address,<br>> > <br>> > Yes, well found. I prefer the following fix however.<br>> > <br>> > - Joe<br>> <br>> It seems ok.<br>> <br>> The patch is too big, I think the only change that needs to be done to fix <br>> the bug is to replace "struct task_struct *sleeper;" with "struct <br>> completion *completion;", replace "if (io->sleeper) <br>> wake_up_process(io->sleeper);" with "if (io->completion) <br>> complete(io->completion);" and declare the completion in sync_io() and <br>> wait on it instead of "while (1)" loop there.<br><br>Here is the minimalist fix you suggested (I agree that splitting out a<br>minimalist fix is useful):<br><br> drivers/md/dm-io.c | 22 ++++++++--------------<br> 1 file changed, 8 insertions(+), 14 deletions(-)<br><br>diff --git a/drivers/md/dm-io.c b/drivers/md/dm-io.c<br>index 2a20986..e60c2ea 100644<br>--- a/drivers/md/dm-io.c<br>+++ b/drivers/md/dm-io.c<br>@@ -10,6 +10,7 @@<br> #include <linux/device-mapper.h><br> <br> #include <linux/bio.h><br>+#include <linux/completion.h><br> #include <linux/mempool.h><br> #include <linux/module.h><br> #include <linux/sched.h><br>@@ -32,7 +33,7 @@ struct dm_io_client {<br> struct io {<br> unsigned long error_bits;<br> atomic_t count;<br>- struct task_struct *sleeper;<br>+ struct completion *wait;<br> struct dm_io_client *client;<br> io_notify_fn callback;<br> void *context;<br>@@ -121,8 +122,8 @@ static void dec_count(struct io *io, unsigned int region, int error)<br> invalidate_kernel_vmap_range(io->vma_invalidate_address,<br> io->vma_invalidate_size);<br> <br>- if (io->sleeper)<br>- wake_up_process(io->sleeper);<br>+ if (io->wait)<br>+ complete(io->wait);<br> <br> else {<br> unsigned long r = io->error_bits;<br>@@ -385,6 +386,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,<br> */<br> volatile char io_[sizeof(struct io) + __alignof__(struct io) - 1];<br> struct io *io = (struct io *)PTR_ALIGN(&io_, __alignof__(struct io));<br>+ DECLARE_COMPLETION_ONSTACK(wait);<br> <br> if (num_regions > 1 && (rw & RW_MASK) != WRITE) {<br> WARN_ON(1);<br>@@ -393,7 +395,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,<br> <br> io->error_bits = 0;<br> atomic_set(&io->count, 1); /* see dispatch_io() */<br>- io->sleeper = current;<br>+ io->wait = &wait;<br> io->client = client;<br> <br> io->vma_invalidate_address = dp->vma_invalidate_address;<br>@@ -401,15 +403,7 @@ static int sync_io(struct dm_io_client *client, unsigned int num_regions,<br> <br> dispatch_io(rw, num_regions, where, dp, io, 1);<br> <br>- while (1) {<br>- set_current_state(TASK_UNINTERRUPTIBLE);<br>-<br>- if (!atomic_read(&io->count))<br>- break;<br>-<br>- io_schedule();<br>- }<br>- set_current_state(TASK_RUNNING);<br>+ wait_for_completion_io(&wait);<br> <br> if (error_bits)<br> *error_bits = io->error_bits;<br>@@ -432,7 +426,7 @@ static int async_io(struct dm_io_client *client, unsigned int num_regions,<br> io = mempool_alloc(client->pool, GFP_NOIO);<br> io->error_bits = 0;<br> atomic_set(&io->count, 1); /* see dispatch_io() */<br>- io->sleeper = NULL;<br>+ io->wait = NULL;<br> io->client = client;<br> io->callback = fn;<br> io->context = context;<br></blockquote>