Message-ID: <87edgzeoez.fsf@oracle.com>
Date:   Wed, 08 Nov 2023 16:55:00 -0800
From:   Ankur Arora <ankur.a.arora@...cle.com>
To:     Chris Packham <Chris.Packham@...iedtelesis.co.nz>
Cc:     Ankur Arora <ankur.a.arora@...cle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
        "paulmck@...nel.org" <paulmck@...nel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "luto@...nel.org" <luto@...nel.org>, "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "juri.lelli@...hat.com" <juri.lelli@...hat.com>,
        "vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
        "willy@...radead.org" <willy@...radead.org>,
        "mgorman@...e.de" <mgorman@...e.de>,
        "jon.grimm@....com" <jon.grimm@....com>,
        "bharata@....com" <bharata@....com>,
        "raghavendra.kt@....com" <raghavendra.kt@....com>,
        "boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
        "konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
        "jgross@...e.com" <jgross@...e.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "bristot@...nel.org" <bristot@...nel.org>,
        "mathieu.desnoyers@...icios.com" <mathieu.desnoyers@...icios.com>,
        "geert@...ux-m68k.org" <geert@...ux-m68k.org>,
        "glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
        "anton.ivanov@...bridgegreys.com" <anton.ivanov@...bridgegreys.com>,
        "mattst88@...il.com" <mattst88@...il.com>,
        "krypton@...ich-teichert.org" <krypton@...ich-teichert.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "David.Laight@...LAB.COM" <David.Laight@...LAB.COM>,
        "richard@....at" <richard@....at>,
        "mjguzik@...il.com" <mjguzik@...il.com>,
        Oded Gabbay <ogabbay@...nel.org>,
        Miguel Ojeda <ojeda@...nel.org>, Jens Axboe <axboe@...nel.dk>,
        Minchan Kim <minchan@...nel.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        "Sudip Mukherjee" <sudipm.mukherjee@...il.com>,
        Theodore Ts'o <tytso@....edu>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Amit Shah <amit@...nel.org>, Gonglei <arei.gonglei@...wei.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        "Davidlohr Bueso" <dave@...olabs.net>,
        Jonathan Cameron <jonathan.cameron@...wei.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Alison Schofield <alison.schofield@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        "Ira Weiny" <ira.weiny@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        "Sumit Semwal" <sumit.semwal@...aro.org>,
        Christian König <christian.koenig@....com>,
        Andi Shyti <andi.shyti@...nel.org>,
        Ray Jui <rjui@...adcom.com>,
        Scott Branden <sbranden@...adcom.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Junxian Huang <huangjunxian6@...ilicon.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Will Deacon <will@...nel.org>, Joerg Roedel <joro@...tes.org>,
        "Mauro Carvalho Chehab" <mchehab@...nel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Mark Gross <markgross@...nel.org>,
        Finn Thain <fthain@...ux-m68k.org>,
        Michael Schmitz <schmitzmic@...il.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Kashyap Desai <kashyap.desai@...adcom.com>,
        Sumit Saxena <sumit.saxena@...adcom.com>,
        Shivasharan S <shivasharan.srikanteshwara@...adcom.com>,
        Mark Brown <broonie@...nel.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        "Jens Wiklander" <jens.wiklander@...aro.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Helge Deller <deller@....de>,
        David Hildenbrand <david@...hat.com>
Subject: Re: [RFC PATCH 85/86] treewide: drivers: remove cond_resched()


Chris Packham <Chris.Packham@...iedtelesis.co.nz> writes:

> On 8/11/23 12:08, Ankur Arora wrote:
>> There are broadly three sets of uses of cond_resched():
>>
>> 1.  Calls to cond_resched() out of the goodness of our heart,
>>      otherwise known as avoiding lockup splats.
>>
>> 2.  Open coded variants of cond_resched_lock() which call
>>      cond_resched().
>>
>> 3.  Retry or error handling loops, where cond_resched() is used as a
>>      quick alternative to spinning in a tight-loop.
>>
>> When running under a full preemption model, the cond_resched() reduces
>> to a NOP (not even a barrier) so removing it obviously cannot matter.
>>
>> But considering only voluntary preemption models (for say code that
>> has been mostly tested under those), for set-1 and set-2 the
>> scheduler can now preempt kernel tasks running beyond their time
>> quanta anywhere they are preemptible() [1]. Which removes any need
>> for these explicitly placed scheduling points.
>>
>> The cond_resched() calls in set-3 are a little more difficult.
>> To start with, given it's NOP character under full preemption, it
>> never actually saved us from a tight loop.
>> With voluntary preemption, it's not a NOP, but it might as well be --
>> for most workloads the scheduler does not have an interminable supply
>> of runnable tasks on the runqueue.
>>
>> So, cond_resched() is useful to not get softlockup splats, but not
>> terribly good for error handling. Ideally, these should be replaced
>> with some kind of timed or event wait.
>> For now we use cond_resched_stall(), which tries to schedule if
>> possible, and executes a cpu_relax() if not.
>>
>> The cond_resched() calls here are all kinds. Those from set-1
>> or set-2 are quite straight-forward to handle.
>>
>> There are quite a few from set-3, where as noted above, we
>> use cond_resched() as if it were a amulent. Which I supppose
>> it is, in that it wards off softlockup or RCU splats.
>>
>> Those are now cond_resched_stall(), but in most cases, given
>> that the timeouts are in milliseconds, they could be easily
>> timed waits.
>
> For i2c-mpc.c:
>
> It looks as the code in question could probably be converted to
> readb_poll_timeout(). If I find sufficient round-tuits I might look at
> that. Regardless in the context of the tree-wide change ...
>
> Reviewed-by: Chris Packham <chris.packham@...iedtelesis.co.nz>

Thanks Chris. This'll take a while before this lands.
I'll see if I can send a patch with cond_resched_stall() or similar
separately.

Meanwhile please feel free to make the readb_poll_timeout() change.

--
ankur

Powered by blists - more mailing lists