<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
<p style="margin-top:0;margin-bottom:0"><span style="font-size: 12pt;">You mean the caller must guarantees the excl fence will only signal till all shared fence signaled, so you can just </span><br>
</p>
<p style="margin-top:0;margin-bottom:0">ignores all shared fence if  add_excl_fence() is invoked.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">e.g. the excl fence is with the same ctx (but later) of the one in shared list.</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">thanks for the explanation </p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0">/Monk</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
<p style="margin-top:0;margin-bottom:0"><br>
</p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Koenig, Christian<br>
<b>Sent:</b> Tuesday, March 6, 2018 7:11:50 PM<br>
<b>To:</b> Liu, Monk<br>
<b>Cc:</b> dri-devel@lists.freedesktop.org; Chris Wilson<br>
<b>Subject:</b> Re: reservation questions</font>
<div> </div>
</div>
<div style="background-color:#FFFFFF">
<div class="x_moz-cite-prefix">Hi Monk,<br>
<br>
your check isn't correct because you still haven't understood the semantics here.<br>
<br>
<blockquote type="cite">
<div>the assumption that all shared fences should be signaled before adding excl fence looks not 100% guaranteed in LKG, </div>
</blockquote>
The semantic is NOT that all shared fences are signaled when the exclusive fence is added.<br>
<br>
Instead the requirement is that the exclusive fence signals after all shared fences signaled. In other words that is an asynchronous handling here.<br>
<br>
I honestly don't know how else to explain it.<br>
<br>
Regards,<br>
Christian.<br>
<br>
Am 06.03.2018 um 12:03 schrieb Liu, Monk:<br>
</div>
<blockquote type="cite"><style type="text/css" style="display:none">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
</style>
<div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0; margin-bottom:0">Hi Christian</p>
<p style="margin-top:0; margin-bottom:0"><br>
</p>
<p style="margin-top:0; margin-bottom:0">I use blow patch to capture the incorrect case :</p>
<p style="margin-top:0; margin-bottom:0"><br>
</p>
<div>@@ -267,12 +267,21 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,</div>
<div>        write_seqcount_end(&obj->seq);</div>
<div>        preempt_enable();</div>
<div> </div>
<div>-       /* inplace update, no shared fences */</div>
<div>-       while (i--)</div>
<div>-               dma_fence_put(rcu_dereference_protected(old->shared[i],</div>
<div>-                                               reservation_object_held(obj)));</div>
<div>+       /* inplace update, no shared fences continue after all shared signaled */</div>
<div>+       while (i--) {</div>
<div>+               struct dma_fence *f = rcu_dereference_protected(old->shared[i],</div>
<div>+                                               reservation_object_held(obj));</div>
<div>+               if (!dma_fence_is_signaled(f))</div>
<div>+                       BUG();</div>
<div>+</div>
<div>+               dma_fence_put(f);</div>
<div>+               /* better assign shared[i] with NULL for sure */</div>
<div>+               rcu_assign_pointer(old->shared[i], NULL);</div>
<div>+       }</div>
<div> </div>
<div>        dma_fence_put(old_fence);</div>
<div>+</div>
<div>+</div>
<div> }</div>
<div> EXPORT_SYMBOL(reservation_object_add_excl_fence);</div>
<div><br>
</div>
<div>and I hit this BUG() during test:</div>
<div><br>
</div>
<div>
<div>[  105.244816] [drm] Initialized amdgpu 3.24.0 20150101 for 0000:00:08.0 on minor 0</div>
<div>[  105.623332] ------------[ cut here ]------------</div>
<div>[  105.623335] kernel BUG at drivers/dma-buf/reservation.c:275!</div>
<div>[  105.624470] invalid opcode: 0000 [#1] SMP</div>
<div>[  105.624915] Modules linked in: amdgpu chash gpu_sched ttm drm_kms_helper drm i2c_algo_bit fb_sys_fops syscopyarea sysfillrect sysimgblt snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep crct10dif_pclmul crc32_pclmul snd_pcm ghash_clmulni_intel
 pcbc snd_seq_midi snd_seq_midi_event snd_rawmidi aesni_intel aes_x86_64 crypto_simd glue_helper cryptd snd_seq snd_seq_device snd_timer serio_raw snd soundcore i2c_piix4 mac_hid parport_pc ppdev lp parport autofs4 8139too psmouse 8139cp mii floppy pata_acpi</div>
<div>[  105.630547] CPU: 3 PID: 1216 Comm: 3dmark Not tainted 4.13.0-debug #1</div>
<div>[  105.631762] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014</div>
<div>[  105.633528] task: ffff8f8a6a165a00 task.stack: ffffb1204159c000</div>
<div>[  105.634676] RIP: 0010:reservation_object_add_excl_fence+0x9c/0xf0</div>
<div>[  105.635824] RSP: 0018:ffffb1204159f9f0 EFLAGS: 00010246</div>
<div>[  105.636805] RAX: 0000000000000000 RBX: ffff8f8a64bee760 RCX: ffff8f8a6bfa2f50</div>
<div>[  105.638123] RDX: ffff8f8a6bfa6770 RSI: ffff8f8a64bee660 RDI: ffff8f8a6635f628</div>
<div>[  105.639440] RBP: ffffb1204159fa18 R08: 0000000000000000 R09: 0000000000000001</div>
<div>[  105.640702] R10: ffffb1204159f808 R11: 0000000000000003 R12: 0000000000000000</div>
<div>[  105.641947] R13: 0000000000000000 R14: ffff8f8a6d0f0200 R15: ffff8f8a64beee60</div>
<div>[  105.643165] FS:  00007fd13c73d940(0000) GS:ffff8f8a76d80000(0000) knlGS:0000000000000000</div>
<div>[  105.644573] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033</div>
<div>[  105.646482] CR2: 00007fd13c6fd000 CR3: 00000001a2a58000 CR4: 00000000001406e0</div>
<div>[  105.648467] Call Trace:</div>
<div>[  105.652480]  amdgpu_bo_do_create+0x3a1/0x540 [amdgpu]</div>
<div>[  105.654233]  amdgpu_bo_create+0x3a/0x220 [amdgpu]</div>
<div>[  105.655956]  amdgpu_vm_alloc_levels.isra.14+0x1dc/0x370 [amdgpu]</div>
<div>[  105.657641]  amdgpu_vm_alloc_pts+0x49/0x70 [amdgpu]</div>
<div>[  105.659155]  amdgpu_gem_va_ioctl+0x365/0x520 [amdgpu]</div>
<div>[  105.660698]  ? amdgpu_gem_create_ioctl+0x19a/0x280 [amdgpu]</div>
<div>[  105.662515]  ? amdgpu_gem_metadata_ioctl+0x1c0/0x1c0 [amdgpu]</div>
<div>[  105.664203]  drm_ioctl_kernel+0x69/0xb0 [drm]</div>
<div>[  105.665491]  ? drm_ioctl_kernel+0x69/0xb0 [drm]</div>
<div>[  105.666959]  drm_ioctl+0x2d2/0x390 [drm]</div>
<div>[  105.668373]  ? amdgpu_gem_metadata_ioctl+0x1c0/0x1c0 [amdgpu]</div>
<div>[  105.670056]  ? call_rcu_sched+0x1d/0x20</div>
<div>[  105.671516]  ? put_object+0x26/0x30</div>
<div>[  105.672741]  ? __delete_object+0x39/0x50</div>
<div>[  105.674048]  amdgpu_drm_ioctl+0x4c/0x80 [amdgpu]</div>
<div>[  105.675551]  do_vfs_ioctl+0x92/0x5a0</div>
<div>[  105.676874]  ? kvm_sched_clock_read+0x1e/0x30</div>
<div>[  105.678276]  ? sched_clock+0x9/0x10</div>
<div>[  105.679553]  ? get_vtime_delta+0x99/0xc0</div>
<div>[  105.681007]  SyS_ioctl+0x79/0x90</div>
<div>[  105.684574]  do_syscall_64+0x6e/0x150</div>
<div>[  105.685910]  entry_SYSCALL64_slow_path+0x25/0x25</div>
<div>[  105.687354] RIP: 0033:0x7fd13b25ff47</div>
<div>[  105.688666] RSP: 002b:00007fff5422b2c8 EFLAGS: 00000202 ORIG_RAX: 0000000000000010</div>
<div>[  105.691268] RAX: ffffffffffffffda RBX: 0000000001886130 RCX: 00007fd13b25ff47</div>
<div>[  105.693148] RDX: 00007fff5422b390 RSI: 00000000c0286448 RDI: 0000000000000007</div>
<div>[  105.695003] RBP: 00007fff5422b300 R08: 0000000300000000 R09: 000000000000000e</div>
<div>[  105.696774] R10: 0000000001887c28 R11: 0000000000000202 R12: 000000000188a430</div>
<div>[  105.698459] R13: 0000000001886130 R14: 00007fff5422b638 R15: 0000000000000000</div>
<div>[  105.700168] Code: 74 3f 41 89 c4 45 89 e5 4b 8b 5c ee 18 48 8b 43 48 a8 01 75 cc 48 8b 43 08 48 8b 40 18 48 85 c0 74 09 48 89 df ff d0 84 c0 75 0c <0f> 0b 48 89 df e8 2a ed ff ff eb b4 48 89 df e8 80 ef ff ff eb </div>
<div>[  105.704982] RIP: reservation_object_add_excl_fence+0x9c/0xf0 RSP: ffffb1204159f9f0</div>
<div><br>
</div>
<br>
</div>
<div>the assumption that all shared fences should be signaled before adding excl fence looks not 100% guaranteed in LKG, </div>
<p style="margin-top:0; margin-bottom:0">Going to take a deep look ...</p>
<p style="margin-top:0; margin-bottom:0"><br>
</p>
<p style="margin-top:0; margin-bottom:0">/Monk</p>
<br>
<br>
<div style="color:rgb(0,0,0)">
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Liu, Monk<br>
<b>Sent:</b> Tuesday, March 6, 2018 6:47 PM<br>
<b>To:</b> Koenig, Christian; Chris Wilson; <a class="x_moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">
dri-devel@lists.freedesktop.org</a><br>
<b>Subject:</b> Re: reservation questions</font>
<div> </div>
</div>
<div dir="ltr">
<div id="x_x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0; margin-bottom:0">ok, that's good point ... </p>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Koenig, Christian<br>
<b>Sent:</b> Tuesday, March 6, 2018 6:42:44 PM<br>
<b>To:</b> Liu, Monk; Chris Wilson; <a class="x_moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">
dri-devel@lists.freedesktop.org</a><br>
<b>Subject:</b> Re: reservation questions</font>
<div> </div>
</div>
<div style="background-color:#FFFFFF">
<div class="x_x_x_moz-cite-prefix">Hi Monk,<br>
<br>
that is to remove the problem that allocating memory could fail.<br>
<br>
E.g. we only add the fence after sending the command to the hardware, so there is now way back and we need to add the fence or break memory management.<br>
<br>
So reservation_object_reserve_shared() makes sure there is a free fence slot *before* we start to prepare things for the hardware.<br>
<br>
Regards,<br>
Christian.<br>
<br>
Am 06.03.2018 um 11:19 schrieb Liu, Monk:<br>
</div>
<blockquote type="cite"><style type="text/css" style="display:none">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
</style>
<div id="x_x_x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0; margin-bottom:0">Hi Chris<span style="font-size:12pt"> </span></p>
<p style="margin-top:0; margin-bottom:0"><span style="font-size:12pt"><br>
</span></p>
<p style="margin-top:0; margin-bottom:0">another question is why we not just call "<span style="color:rgb(220,220,170); background-color:rgb(30,30,30); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; white-space:pre">reservation_object_reserve_shared"</span></p>
<p style="margin-top:0; margin-bottom:0"><span style="color:rgb(0,0,0); background-color:rgb(255,255,255); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; white-space:pre">during below add_shared_fence function, so the
 BUG_ON() could be avoided and caller won't need</span></p>
<p style="margin-top:0; margin-bottom:0"><span style="color:rgb(0,0,0); background-color:rgb(255,255,255); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; white-space:pre">to worry when and how much time it should call
 reserve_shared() ?</span></p>
<p style="margin-top:0; margin-bottom:0"><span style="color:rgb(0,0,0); background-color:rgb(255,255,255); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; white-space:pre"></span></p>
<p style="margin-top:0; margin-bottom:0"><span style="color:rgb(0,0,0); background-color:rgb(255,255,255); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; white-space:pre">thanks !</span></p>
<p style="margin-top:0; margin-bottom:0"><span style="font-size:12pt"><br>
</span></p>
<p style="margin-top:0; margin-bottom:0"><span style="font-size:12pt"></span></p>
<div style="color:rgb(212,212,212); background-color:rgb(30,30,30); font-family:"Droid Sans Mono",monospace,monospace,"Droid Sans Fallback"; font-size:14px; line-height:19px; white-space:pre">
<div><span style="color:#569cd6">void</span> <span style="color:#dcdcaa">reservation_object_add_shared_fence</span>(<span style="color:#569cd6">struct</span> reservation_object *obj,</div>
<div>                     <span style="color:#569cd6">struct</span> dma_fence *fence)</div>
<div>{</div>
<div>    <span style="color:#569cd6">struct</span> reservation_object_list *old, *fobj = obj-><span style="color:#9cdcfe">staged</span>;</div>
<div>    old = <span style="color:#dcdcaa">reservation_object_get_list</span>(obj);</div>
<div>    obj-><span style="color:#9cdcfe">staged</span> = <span style="color:#569cd6">
NULL</span>;</div>
<div>    <span style="color:#c586c0">if</span> (!fobj) {</div>
<div>        <span style="color:#dcdcaa">BUG_ON</span>(old-><span style="color:#9cdcfe">shared_count</span> >= old-><span style="color:#9cdcfe">shared_max</span>);</div>
<div>        <span style="color:#dcdcaa">reservation_object_add_shared_inplace</span>(obj, old, fence);</div>
<div>    } <span style="color:#c586c0">else</span></div>
<div>        <span style="color:#dcdcaa">reservation_object_add_shared_replace</span>(obj, old, fobj, fence);</div>
<div>}</div>
<div><span style="color:#dcdcaa">EXPORT_SYMBOL</span>(reservation_object_add_shared_fence);</div>
</div>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_x_x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Chris Wilson
<a class="x_x_x_moz-txt-link-rfc2396E" href="mailto:chris@chris-wilson.co.uk"><chris@chris-wilson.co.uk></a><br>
<b>Sent:</b> Tuesday, March 6, 2018 6:10:21 PM<br>
<b>To:</b> Liu, Monk; <a class="x_x_x_moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">
dri-devel@lists.freedesktop.org</a>; Koenig, Christian<br>
<b>Subject:</b> Re: reservation questions</font>
<div> </div>
</div>
<div class="x_x_x_BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="x_x_x_PlainText">Quoting Liu, Monk (2018-03-06 09:45:19)<br>
> call reservation_object_add_excl_fence,<br>
> it set obj->fence->shared_count to 0, and put all shared fence from obj->fence<br>
> without waiting signaling.<br>
> (this action looks inappropriate, I think at least before put all those shared<br>
> fences<br>
> we should dma_wait_fence() on them to make sure they are signaled)<br>
<br>
No. Serialisation of resv updates are handled by the caller, the fences<br>
are ordered asynchronously so the wait is implicit in the construction.<br>
(I.e. before the excl fence can be signaled, all of the earlier shared<br>
fences must be signaled. You can even say before the operation that the<br>
excl fence signals completion of can begin, all the shared fences must<br>
have been signaled. But that is all implicit so that we can do it<br>
asynchronously.)<br>
<br>
> call reservation_object_reserve_shared,<br>
> this time obj->staged isn't NULL, and it is freed (nothing bad now<br>
> since obj->fence points to other place),<br>
> and obj->staged set to NULL,<br>
> <br>
> call reservation_object_add_shared_fence,<br>
> this time should going through reservation_object_add_shared_inplace,<br>
> But BUG_ON(old->shared_count >= old->shared_max) will hit !<br>
<br>
How? You only free staged iff shared_count < shared_max.<br>
<br>
You've reminded me that we should cover all this with a bunch of<br>
selftests.<br>
-Chris<br>
</div>
</span></font></div>
</blockquote>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</body>
</html>