<div dir="ltr"><br><div class="gmail_extra">Dear Lars,</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thank you for your kind reply.~</div><div class="gmail_extra"><br></div><div class="gmail_extra">Frankly speaking, I don't understand your comments clearly yet. </div><div class="gmail_extra">Let me check deeply our modification code, later.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Anyway, I think you might have meet this situation if you did CASE-14 test.</div><div class="gmail_extra">Could you please test "CASE-14 primary node hang by VM-net-disconnect during big file copy".</div><div class="gmail_extra">I think it's critical problem.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I will wait for your good news.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks.</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br><div class="gmail_quote">2016-02-15 22:54 GMT+09:00 Lars Ellenberg <span dir="ltr"><<a href="mailto:lars.ellenberg@linbit.com" target="_blank">lars.ellenberg@linbit.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>On Sun, Feb 14, 2016 at 07:39:36PM +0900, 김재헌 wrote:<br>
> Dear Philipp,<br>
><br>
> Please check my previous question of CASE-14("[DRBD-user] [CASE-14] primary<br>
> node hang by VM-net-disconnect during big file copy").<br>
> According to this case, Linux drbd deadlock may occur.<br>
> On the other hand, Windows side there is no deadlock but sometimes the<br>
> transfer_log list is broken in _tl_restart function.<br>
><br>
> So, We are trying to modify the source code as follows:<br>
><br>
> 1. Modifications<br>
><br>
> 1) in drbd_send_and_submit()<br>
><br>
> if (likely(req->i.size != 0)) {<br>
> if (rw == WRITE) {<br>
> struct drbd_request *req2;<br>
> resource->current_tle_writes++;<br>
> #if 0 // WIN32 ### ignore tail_recursion ###<br>
> list_for_each_entry_reverse(req2, &resource->transfer_log, tl_requests) {<br>
> if (req2->rq_state[0] & RQ_WRITE) {<br>
> /* Make the new write request depend on<br>
> * the previous one. */<br>
> kref_get(&req->kref);<br>
> break;<br>
> }<br>
> }<br>
> #endif<br>
> }<br>
><br>
> list_add_tail(&req->tl_requests, &resource->transfer_log);<br>
> }<br>
><br>
><br>
> 2) in drbd_req_destroy()<br>
><br>
> if (s & RQ_WRITE && req_size) {<br>
> list_for_each_entry(req, &device->resource->transfer_log, tl_requests) {<br>
> if (req->rq_state[0] & RQ_WRITE) {<br>
> /*<br>
> * Do the equivalent of:<br>
> * kref_put(&req->kref, drbd_req_destroy)<br>
> * without recursing into the destructor.<br>
> */<br>
> #if 0 // WIN32 ### ignore tail_recursion ###<br>
> if (atomic_dec_and_test(&req->kref.refcount))<br>
> goto tail_recursion;<br>
> #endif<br>
> break;<br>
> }<br>
> }<br>
> }<br>
><br>
><br>
> 2. Questions<br>
><br>
> 1) This part of "tail_recursion" is a new design on verson 9.<br>
> Is this essential operation?<br>
> I mean, what do you think about my ignoring tail_recursion part for<br>
> temporary workaround?<br>
<br>
</div></div>I cannot explain all the implications within two lines of text, but we<br>
want the "destruction" of drbd_requests to happen in the order they have<br>
been put on the transfer log.<br>
That is important in some multi-peer scenarios.<br>
<br>
If there is no explicit dependency (here implemented via kref), they<br>
could be destroyed out-of-order, and that could lead to bad decisions<br>
elsewhere, or potentially not enough being resynced in some scenarios.<br>
<span><br>
> 2) And what is the reason for the marking of "kref_get(&req->kref);" in<br>
> drbd_send_and_submit and processing with recursion in drbd_req_destroy<br>
> later?<br>
<br>
</span>see above.<br>
<span><br>
> 3) On Windows side, we ignore this part(see source code of "#if 0 // WIN32<br>
> ### ignore tail_recursion ###").<br>
> Anyway, after ignore, Windows drbd engine works well, till now. Is<br>
> there any problem?<br>
<br>
</span>see above.<br>
<span><br>
> On Linux side, you cannot see this list-crash-case because the CASE-14 test<br>
> may be done by deadlock first.<br>
> Please check the CASE-14 deadlock case first and then check this CASE-20.<br>
<br>
</span>Cheers,<br>
<br>
Lars<br>
<br>
</blockquote></div><div><div dir="ltr"><div><div dir="ltr"><p style="margin:0cm 0cm 0.0001pt;line-height:21px"><span lang="EN-US" style="font-size:8pt;line-height:16px;font-family:'\00b9d1\00c740 \00ace0\00b515';color:rgb(127,127,127)"><br></span></p></div></div></div></div>
</div></div>