<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">
<blockquote type="cite">
Why we break out the loops when there are pending bos to be
released?</blockquote>
<br>
We do this anyway if we can't acquire the necessary locks. Freeing
already deleted BOs is just a very lazy background work.<br>
<br>
<blockquote type="cite">
So it did not break anything with this patch I think.</blockquote>
<br>
Oh, the patch will certainly work. I'm just not sure if it's the
ideal behavior.<br>
<br>
<blockquote type="cite">
<div dir="auto" style="direction: ltr; margin: 0; padding: 0;
font-family: sans-serif; font-size: 11pt; color: black; ">
<a class="moz-txt-link-freetext" href="https://elixir.bootlin.com/linux/latest/source/mm/slab.c#L4026">https://elixir.bootlin.com/linux/latest/source/mm/slab.c#L4026</a><br>
<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0;
font-family: sans-serif; font-size: 11pt; color: black; ">
This is another example of the usage of cond_sched.</div>
</blockquote>
<br>
Yes, and that is also a good example of what I mean here:<br>
<br>
<blockquote type="cite">
<pre> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n"><a href="https://elixir.bootlin.com/linux/latest/ident/mutex_trylock">mutex_trylock</a></span><span class="p">(</span><span class="o">&</span><span class="n"><a href="https://elixir.bootlin.com/linux/latest/ident/slab_mutex">slab_mutex</a></span><span class="p">))</span>
<span class="cm">/* Give up. Setup the next iteration. */</span>
<span class="k">goto</span> <span class="n">out</span><span class="p">;</span></pre>
</blockquote>
<br>
If the function can't acquire the lock immediately it gives up and
waits for the next iteration.<br>
<br>
I think it would be better if we do this in TTM as well if we
spend to much time cleaning up old BOs.<br>
<br>
On the other hand you are right that cond_resched() has the
advantage that we could spend more time on cleaning up old BOs if
there is nothing else for the CPU TODO.<br>
<br>
Regards,<br>
Christian.<br>
<br>
Am 09.04.20 um 16:24 schrieb Pan, Xinhui:<br>
</div>
<blockquote type="cite" cite="mid:BL0PR12MB2547BF609F21C48B17A402A087C10@BL0PR12MB2547.namprd12.prod.outlook.com">
<div dir="auto" style="direction: ltr; margin: 0; padding: 0;
font-family: sans-serif; font-size: 11pt; color: black; ">
<a class="moz-txt-link-freetext" href="https://elixir.bootlin.com/linux/latest/source/mm/slab.c#L4026">https://elixir.bootlin.com/linux/latest/source/mm/slab.c#L4026</a><br>
<br>
</div>
<div dir="auto" style="direction: ltr; margin: 0; padding: 0;
font-family: sans-serif; font-size: 11pt; color: black; ">
This is another example of the usage of cond_sched.</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Pan,
Xinhui <a class="moz-txt-link-rfc2396E" href="mailto:Xinhui.Pan@amd.com"><Xinhui.Pan@amd.com></a><br>
<b>Sent:</b> Thursday, April 9, 2020 10:11:08 PM<br>
<b>To:</b> Lucas Stach <a class="moz-txt-link-rfc2396E" href="mailto:l.stach@pengutronix.de"><l.stach@pengutronix.de></a>;
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a>; Koenig, Christian
<a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a><br>
<b>Cc:</b> <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:dri-devel@lists.freedesktop.org"><dri-devel@lists.freedesktop.org></a><br>
<b>Subject:</b> Re: [PATCH] drm/ttm: Schedule out if possibe
in bo delayed delete worker</font>
<div> </div>
</div>
<div>
<div dir="auto" style="direction:ltr; margin:0; padding:0;
font-family:sans-serif; font-size:11pt; color:black">
I think it doesn't matter if workitem schedule out. Even we
did not schedule out, the workqueue itself will schedule out
later.<br>
</div>
<div dir="auto" style="direction:ltr; margin:0; padding:0;
font-family:sans-serif; font-size:11pt; color:black">
So it did not break anything with this patch I think.</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Pan,
Xinhui <a class="moz-txt-link-rfc2396E" href="mailto:Xinhui.Pan@amd.com"><Xinhui.Pan@amd.com></a><br>
<b>Sent:</b> Thursday, April 9, 2020 10:07:09 PM<br>
<b>To:</b> Lucas Stach <a class="moz-txt-link-rfc2396E" href="mailto:l.stach@pengutronix.de"><l.stach@pengutronix.de></a>;
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a>; Koenig, Christian
<a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a><br>
<b>Cc:</b> <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:dri-devel@lists.freedesktop.org"><dri-devel@lists.freedesktop.org></a><br>
<b>Subject:</b> Re: [PATCH] drm/ttm: Schedule out if possibe
in bo delayed delete worker</font>
<div> </div>
</div>
<div>
<div dir="auto" style="direction:ltr; margin:0; padding:0;
font-family:sans-serif; font-size:11pt; color:black">
Why we break out the loops when there are pending bos to be
released?<br>
<br>
</div>
<div dir="auto" style="direction:ltr; margin:0; padding:0;
font-family:sans-serif; font-size:11pt; color:black">
And I just checked the process_one_work. Right after the
work item callback is called, the workqueue itself will
call cond_resched. So I think
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_x_divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Koenig, Christian
<a class="moz-txt-link-rfc2396E" href="mailto:Christian.Koenig@amd.com"><Christian.Koenig@amd.com></a><br>
<b>Sent:</b> Thursday, April 9, 2020 9:38:24 PM<br>
<b>To:</b> Lucas Stach <a class="moz-txt-link-rfc2396E" href="mailto:l.stach@pengutronix.de"><l.stach@pengutronix.de></a>;
Pan, Xinhui <a class="moz-txt-link-rfc2396E" href="mailto:Xinhui.Pan@amd.com"><Xinhui.Pan@amd.com></a>;
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a><br>
<b>Cc:</b> <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:dri-devel@lists.freedesktop.org"><dri-devel@lists.freedesktop.org></a><br>
<b>Subject:</b> Re: [PATCH] drm/ttm: Schedule out if
possibe in bo delayed delete worker</font>
<div> </div>
</div>
<div class="x_x_BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="x_x_PlainText">Am 09.04.20 um 15:25 schrieb
Lucas Stach:<br>
> Am Donnerstag, den 09.04.2020, 14:35 +0200
schrieb Christian König:<br>
>> Am 09.04.20 um 03:31 schrieb xinhui pan:<br>
>>> The delayed delete list is per device
which might be very huge. And in<br>
>>> a heavy workload test, the list might
always not be empty. That will<br>
>>> trigger any RCU stall warnings or
softlockups in non-preemptible kernels<br>
>>> Lets do schedule out if possible in that
case.<br>
>> Mhm, I'm not sure if that is actually
allowed. This is called from a<br>
>> work item and those are not really supposed
to be scheduled away.<br>
> Huh? Workitems can schedule out just fine,
otherwise they would be<br>
> horribly broken when it comes to sleeping locks.<br>
<br>
Let me refine the sentence: Work items are not really
supposed to be <br>
scheduled purposely. E.g. you shouldn't call
schedule() or <br>
cond_resched() like in the case here.<br>
<br>
Getting scheduled away because we wait for a lock is
of course perfectly <br>
fine.<br>
<br>
> The workqueue code<br>
> even has measures to keep the workqueues at the
expected concurrency<br>
> level by starting other workitems when one of
them goes to sleep.<br>
<br>
Yeah, and exactly that's what I would say we should
avoid here :)<br>
<br>
In other words work items can be scheduled away, but
they should not if <br>
not really necessary (e.g. waiting for a lock).<br>
<br>
Otherwise as you said new threads for work item
processing are started <br>
up and I don't think we want that.<br>
<br>
Just returning from the work item and waiting for the
next cycle is most <br>
likely the better option.<br>
<br>
Regards,<br>
Christian.<br>
<br>
><br>
> Regards,<br>
> Lucas<br>
><br>
>> Maybe rather change the while into while
(!list_empty(&bdev->ddestroy)<br>
>> && !should_reschedule(0)).<br>
>><br>
>> Christian.<br>
>><br>
>>> Signed-off-by: xinhui pan
<a class="moz-txt-link-rfc2396E" href="mailto:xinhui.pan@amd.com"><xinhui.pan@amd.com></a><br>
>>> ---<br>
>>> drivers/gpu/drm/ttm/ttm_bo.c | 1 +<br>
>>> 1 file changed, 1 insertion(+)<br>
>>><br>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c<br>
>>> index 9e07c3f75156..b8d853cab33b 100644<br>
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c<br>
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c<br>
>>> @@ -541,6 +541,7 @@ static bool
ttm_bo_delayed_delete(struct ttm_bo_device *bdev, bool
remove_all)<br>
>>> }<br>
>>> <br>
>>> ttm_bo_put(bo);<br>
>>> + cond_resched();<br>
>>>
spin_lock(&glob->lru_lock);<br>
>>> }<br>
>>> list_splice_tail(&removed,
&bdev->ddestroy);<br>
>>
_______________________________________________<br>
>> dri-devel mailing list<br>
>> <a class="moz-txt-link-abbreviated" href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a><br>
>> <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C0a47486676a74702f05408d7dc89839c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637220355504145868&sdata=wbRkYBPI6mYuZjKBtQN3AGLDOwqJlWY3XUtwwSiUQHg%3D&reserved=0" moz-do-not-send="true">
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C0a47486676a74702f05408d7dc89839c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637220355504145868&sdata=wbRkYBPI6mYuZjKBtQN3AGLDOwqJlWY3XUtwwSiUQHg%3D&reserved=0</a><br>
<br>
</div>
</span></font></div>
</div>
</div>
</blockquote>
<br>
</body>
</html>