<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <div dir="auto" style="direction: ltr; margin: 0; padding: 0; font-family: sans-serif; font-size: 11pt; color: black; "> https://elixir.bootlin.com/linux/latest/source/mm/slab.c#L4026<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 face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Pan, Xinhui <Xinhui.Pan@amd.com><br> <b>Sent:</b> Thursday, April 9, 2020 10:11:08 PM<br> <b>To:</b> Lucas Stach <l.stach@pengutronix.de>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com><br> <b>Cc:</b> dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org><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 face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Pan, Xinhui <Xinhui.Pan@amd.com><br> <b>Sent:</b> Thursday, April 9, 2020 10:07:09 PM<br> <b>To:</b> Lucas Stach <l.stach@pengutronix.de>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com><br> <b>Cc:</b> dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org><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 face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Koenig, Christian <Christian.Koenig@amd.com><br> <b>Sent:</b> Thursday, April 9, 2020 9:38:24 PM<br> <b>To:</b> Lucas Stach <l.stach@pengutronix.de>; Pan, Xinhui <Xinhui.Pan@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br> <b>Cc:</b> dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org><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 <xinhui.pan@amd.com><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> >> dri-devel@lists.freedesktop.org<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"> 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> </body> </html>