<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace">Hi Richard,<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">Richard Palethorpe <span dir="ltr"><<a href="mailto:rpalethorpe@suse.de" target="_blank">rpalethorpe@suse.de</a>></span> wrote:<br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<span class="gmail-m_7902868185367280103gmail-"><br>
Li Wang writes:<br>
<br>
> 1. Some children are still working on the read I/O but parent trys to<br>
> stopping them after visit_dir() immediately. Although the stop_attemps<br>
> is 65535, it still sometimes fails, so we get the following worker<br>
> stalled messges in test.<br>
><br>
> # uname -rm<br>
> 4.16.0-rc7 ppc64<br>
> # ./read_all -d /sys -q -r 10<br>
> tst_test.c:987: INFO: Timeout per run is 0h 05m 00s<br>
> read_all.c:280: BROK: Worker 26075 is stalled<br>
> read_all.c:280: WARN: Worker 26075 is stalled<br>
> read_all.c:280: WARN: Worker 26079 is stalled<br>
> read_all.c:280: WARN: Worker 26087 is stalled<br>
<br>
</span>wow, three workers have there queues perfectly filled... I guess I<br>
accidentally created a brute force box packing algorithm.<br>
<span class="gmail-m_7902868185367280103gmail-"><br>
><br>
> 2. The sched_work() push action in a infinite loop, here I propose to let<br>
> it in limited times.<br>
<br>
</span>I think this is moving the problem instead of solving it. Increasing the<br>
number of stop_attempts should have the same effect unless the workers<br>
are permanently blocked on I/O. However this might be better because it<br>
removes the sleep.<br></blockquote><div><br><div style="font-family:monospace,monospace" class="gmail_default">Hmm, not sure if you're fully get my point, maybe I(apologize!) shouldn't fix two<br>problems in one patch. <br><br>For the block I/O issue, I just adding 'usleep(100)' in stop_workers() function.<br></div><div style="font-family:monospace,monospace" class="gmail_default">You suggest increasing stop_attempts is also accessible, but without sleep<br></div><div style="font-family:monospace,monospace" class="gmail_default">it still very fast to finish the loop and probably we need a very large number<br>for stop_attempt to waste time.<br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">For the second change in sched_work() is just to guarantee we can exist the<br>infinite loop if something wrong with queue_push action.<br></div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Possibly we should actually try to determine if a worker is blocked<br>
reading a file and print the file name.<br></blockquote><div><br><div style="font-family:monospace,monospace" class="gmail_default">You are right, I'm now still looking for a better way to avoid this block I/O issue.<br></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_7902868185367280103gmail-"><br>
><br>
> Signed-off-by: Li Wang <<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>><br>
> ---<br>
> testcases/kernel/fs/read_all/r<wbr>ead_all.c | 10 +++++++---<br>
> 1 file changed, 7 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/testcases/kernel/fs/read_all<wbr>/read_all.c b/testcases/kernel/fs/read_all<wbr>/read_all.c<br>
> index b7ed540..ab206e7 100644<br>
> --- a/testcases/kernel/fs/read_all<wbr>/read_all.c<br>
> +++ b/testcases/kernel/fs/read_all<wbr>/read_all.c<br>
> @@ -280,6 +280,7 @@ static void stop_workers(void)<br>
> workers[i].pid);<br>
> break;<br>
> }<br>
> + usleep(100);<div style="font-family:monospace,monospace;display:inline" class="gmail_default"> </div><br>
> }<br>
> }<br>
> }<br>
> @@ -306,9 +307,12 @@ static void sched_work(const char *path)<br>
> if (pushed)<br>
> break;<br>
><br>
> - if (++push_attempts > worker_count) {<br>
> - usleep(100);<br>
> - push_attempts = 0;<br>
> + usleep(100);<br>
> + if (++push_attempts > 0xffff) {<br>
<br>
</span>Maybe add another f to this.<br></blockquote><div><br><div style="font-family:monospace,monospace;display:inline" class="gmail_default">No need too much attempt, my test says this push action can get pass less than try 20 times.<br></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="gmail-m_7902868185367280103gmail-"><br>
> + tst_brk(TBROK,<br>
> + "Attempts %d times but still failed to push %s",<br>
> + push_attempts, path);<br>
> + break;<br>
> }<br>
> }<br>
> }<br>
<br>
<br>
--<br>
</span>Thank you,<br>
Richard.<br>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail-m_7902868185367280103gmail_signature">Li Wang<br><a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a></div>
</div></div>