<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hi Joseph,<br>
      <br>
      On 09/12/2016 09:37 AM, Joseph Qi wrote:<br>
    </div>
    <blockquote cite="mid:57D606EA.5010009@huawei.com" type="cite">
      <pre wrap="">Hi Eric,

On 2016/9/10 17:55, Eric Ren wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">The testcase "mmaptruncate" of ocfs2-test deadlocked occasionally.

In this testcase, we create a 2*CLUSTER_SIZE file and mmap() on it;
there are 2 process repeatedly performing the following operations
respectively: one is doing memset(mmaped_addr + 2*CLUSTER_SIZE - 1,
'a', 1), while the another is playing ftruncate(fd, 2*CLUSTER_SIZE)
and then ftruncate(fd, CLUSTER_SIZE) again and again.

This is the backtrace when the deadlock happens:
[&lt;ffffffff817054f0&gt;] __wait_on_bit_lock+0x50/0xa0
[&lt;ffffffff81199bd7&gt;] __lock_page+0xb7/0xc0
[&lt;ffffffff810c4de0&gt;] ? autoremove_wake_function+0x40/0x40
[&lt;ffffffffa0440f4f&gt;] ocfs2_write_begin_nolock+0x163f/0x1790 [ocfs2]
[&lt;ffffffffa0462a50&gt;] ? ocfs2_allocate_extend_trans+0x180/0x180 [ocfs2]
[&lt;ffffffffa0467b47&gt;] ocfs2_page_mkwrite+0x1c7/0x2a0 [ocfs2]
[&lt;ffffffff811cf286&gt;] do_page_mkwrite+0x66/0xc0
[&lt;ffffffff811d3635&gt;] handle_mm_fault+0x685/0x1350
[&lt;ffffffff81039dc0&gt;] ? __fpu__restore_sig+0x70/0x530
[&lt;ffffffff810694c8&gt;] __do_page_fault+0x1d8/0x4d0
[&lt;ffffffff81069827&gt;] trace_do_page_fault+0x37/0xf0
[&lt;ffffffff81061e69&gt;] do_async_page_fault+0x19/0x70
[&lt;ffffffff8170ac98&gt;] async_page_fault+0x28/0x30

In ocfs2_write_begin_nolock(), we first grab the pages and then
allocate disk space for this write; ocfs2_try_to_free_truncate_log()
will be called if ENOSPC is turned; if we're lucky to get enough clusters,
which is usually the case, we start over again. But in ocfs2_free_write_ctxt()
the target page isn't unlocked, so we will deadlock when trying to grab
the target page again.
</pre>
      </blockquote>
      <pre wrap="">IMO, in ocfs2_grab_pages_for_write, mmap_page is mapping to w_pages and
w_target_locked is set to true, and then will be unlocked by
ocfs2_unlock_pages in ocfs2_free_write_ctxt.
So I'm not getting the case "page isn't unlock". Could you please explain
it in more detail?</pre>
    </blockquote>
    Thanks for review;-) Follow up the calling chain:<br>
    <pre wrap="">ocfs2_free_write_ctxt()
 -&gt; <span class="pl-en">ocfs2_unlock_pages()</span></pre>
    in ocfs2_unlock_pages
    (<a class="moz-txt-link-freetext" href="https://github.com/torvalds/linux/blob/master/fs/ocfs2/aops.c#L793">https://github.com/torvalds/linux/blob/master/fs/ocfs2/aops.c#L793</a>),
    we<br>
    can see the code just put_page(target_page), but not unlock it.<br>
    <br>
    Yeah, I will think this a bit more like:<br>
    why not unlock the target_page there? Is there other potential
    problems if the "ret" is not "-ENOSPC" but<br>
    other possible error code?<br>
    <br>
    Thanks,<br>
    Eric<br>
    <br>
    <blockquote cite="mid:57D606EA.5010009@huawei.com" type="cite">
      <pre wrap="">

Thanks,
Joseph

</pre>
      <blockquote type="cite">
        <pre wrap="">
Fix this issue by unlocking the target page after we fail to allocate
enough space at the first time.

Jan Kara helps me clear out the JBD2 part, and suggest the hint for root cause.

Signed-off-by: Eric Ren <a class="moz-txt-link-rfc2396E" href="mailto:zren@suse.com">&lt;zren@suse.com&gt;</a>
---
 fs/ocfs2/aops.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 98d3654..78d1d67 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -1860,6 +1860,13 @@ out:
                  */
                 try_free = 0;
 
+                /*
+                 * Unlock mmap_page because the page has been locked when we
+                 * are here.
+                 */
+                if (mmap_page)
+                        unlock_page(mmap_page);
+
                 ret1 = ocfs2_try_to_free_truncate_log(osb, clusters_need);
                 if (ret1 == 1)
                         goto try_again;

</pre>
      </blockquote>
      <pre wrap="">


</pre>
    </blockquote>
    <p><br>
    </p>
  </body>
</html>