[RFC PATCH 80/86] treewide: sound: remove cond_resched()

From: Ankur Arora
Date: Tue Nov 07 2023 - 18:13:21 EST


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.

Most uses here are from set-1 when we are executing in extended
loops. Remove them.

In addition there are a few set-3 cases in the neighbourhood of
HW register access. Replace those instances with cond_resched_stall()

[1] https://lore.kernel.org/lkml/20231107215742.363031-1-ankur.a.arora@xxxxxxxxxx/

Cc: Jaroslav Kysela <perex@xxxxxxxx>
Cc: Takashi Iwai <tiwai@xxxxxxxx>
Signed-off-by: Ankur Arora <ankur.a.arora@xxxxxxxxxx>
---
sound/arm/aaci.c | 2 +-
sound/core/seq/seq_virmidi.c | 2 --
sound/hda/hdac_controller.c | 1 -
sound/isa/sb/emu8000_patch.c | 5 -----
sound/isa/sb/emu8000_pcm.c | 2 +-
sound/isa/wss/wss_lib.c | 1 -
sound/pci/echoaudio/echoaudio_dsp.c | 2 --
sound/pci/ens1370.c | 1 -
sound/pci/es1968.c | 2 +-
sound/pci/lola/lola.c | 1 -
sound/pci/mixart/mixart_hwdep.c | 2 +-
sound/pci/pcxhr/pcxhr_core.c | 5 -----
sound/pci/vx222/vx222_ops.c | 2 --
sound/x86/intel_hdmi_audio.c | 1 -
14 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
index 0817ad21af74..d216f4859e61 100644
--- a/sound/arm/aaci.c
+++ b/sound/arm/aaci.c
@@ -145,7 +145,7 @@ static unsigned short aaci_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
timeout = FRAME_PERIOD_US * 8;
do {
udelay(1);
- cond_resched();
+ cond_resched_stall();
v = readl(aaci->base + AACI_SLFR) & (SLFR_1RXV|SLFR_2RXV);
} while ((v != (SLFR_1RXV|SLFR_2RXV)) && --timeout);

diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 1b9260108e48..99226da86d3c 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -154,8 +154,6 @@ static void snd_vmidi_output_work(struct work_struct *work)
if (ret < 0)
break;
}
- /* rawmidi input might be huge, allow to have a break */
- cond_resched();
}
}

diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 7f3a000fab0c..9b6df2f541ca 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -284,7 +284,6 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
msleep(2); /* temporary workaround */
} else {
udelay(10);
- cond_resched();
}
}

diff --git a/sound/isa/sb/emu8000_patch.c b/sound/isa/sb/emu8000_patch.c
index 8c1e7f2bfc34..d808c461be35 100644
--- a/sound/isa/sb/emu8000_patch.c
+++ b/sound/isa/sb/emu8000_patch.c
@@ -218,11 +218,6 @@ snd_emu8000_sample_new(struct snd_emux *rec, struct snd_sf_sample *sp,
offset++;
write_word(emu, &dram_offset, s);

- /* we may take too long time in this loop.
- * so give controls back to kernel if needed.
- */
- cond_resched();
-
if (i == sp->v.loopend &&
(sp->v.mode_flags & (SNDRV_SFNT_SAMPLE_BIDIR_LOOP|SNDRV_SFNT_SAMPLE_REVERSE_LOOP)))
{
diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c
index 9234d4fe8ada..fd18c7cf1812 100644
--- a/sound/isa/sb/emu8000_pcm.c
+++ b/sound/isa/sb/emu8000_pcm.c
@@ -404,7 +404,7 @@ static int emu8k_pcm_trigger(struct snd_pcm_substream *subs, int cmd)
*/
#define CHECK_SCHEDULER() \
do { \
- cond_resched();\
+ cond_resched_stall();\
if (signal_pending(current))\
return -EAGAIN;\
} while (0)
diff --git a/sound/isa/wss/wss_lib.c b/sound/isa/wss/wss_lib.c
index 026061b55ee9..97c74e8c26ee 100644
--- a/sound/isa/wss/wss_lib.c
+++ b/sound/isa/wss/wss_lib.c
@@ -1159,7 +1159,6 @@ static int snd_ad1848_probe(struct snd_wss *chip)
while (wss_inb(chip, CS4231P(REGSEL)) & CS4231_INIT) {
if (time_after(jiffies, timeout))
return -ENODEV;
- cond_resched();
}
spin_lock_irqsave(&chip->reg_lock, flags);

diff --git a/sound/pci/echoaudio/echoaudio_dsp.c b/sound/pci/echoaudio/echoaudio_dsp.c
index 2a40091d472c..085b229c83b5 100644
--- a/sound/pci/echoaudio/echoaudio_dsp.c
+++ b/sound/pci/echoaudio/echoaudio_dsp.c
@@ -100,7 +100,6 @@ static int write_dsp(struct echoaudio *chip, u32 data)
return 0;
}
udelay(1);
- cond_resched();
}

chip->bad_board = true; /* Set true until DSP re-loaded */
@@ -123,7 +122,6 @@ static int read_dsp(struct echoaudio *chip, u32 *data)
return 0;
}
udelay(1);
- cond_resched();
}

chip->bad_board = true; /* Set true until DSP re-loaded */
diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c
index 89210b2c7342..4948ae411a94 100644
--- a/sound/pci/ens1370.c
+++ b/sound/pci/ens1370.c
@@ -501,7 +501,6 @@ static unsigned int snd_es1371_wait_src_ready(struct ensoniq * ensoniq)
r = inl(ES_REG(ensoniq, 1371_SMPRATE));
if ((r & ES_1371_SRC_RAM_BUSY) == 0)
return r;
- cond_resched();
}
dev_err(ensoniq->card->dev, "wait src ready timeout 0x%lx [0x%x]\n",
ES_REG(ensoniq, 1371_SMPRATE), r);
diff --git a/sound/pci/es1968.c b/sound/pci/es1968.c
index 4bc0f53c223b..1598880cfeea 100644
--- a/sound/pci/es1968.c
+++ b/sound/pci/es1968.c
@@ -612,7 +612,7 @@ static int snd_es1968_ac97_wait(struct es1968 *chip)
while (timeout-- > 0) {
if (!(inb(chip->io_port + ESM_AC97_INDEX) & 1))
return 0;
- cond_resched();
+ cond_resched_stall();
}
dev_dbg(chip->card->dev, "ac97 timeout\n");
return 1; /* timeout */
diff --git a/sound/pci/lola/lola.c b/sound/pci/lola/lola.c
index 1aa30e90b86a..3c18b5543512 100644
--- a/sound/pci/lola/lola.c
+++ b/sound/pci/lola/lola.c
@@ -166,7 +166,6 @@ static int rirb_get_response(struct lola *chip, unsigned int *val,
if (time_after(jiffies, timeout))
break;
udelay(20);
- cond_resched();
}
dev_warn(chip->card->dev, "RIRB response error\n");
if (!chip->polling_mode) {
diff --git a/sound/pci/mixart/mixart_hwdep.c b/sound/pci/mixart/mixart_hwdep.c
index 689c0f995a9c..1906cb861002 100644
--- a/sound/pci/mixart/mixart_hwdep.c
+++ b/sound/pci/mixart/mixart_hwdep.c
@@ -41,7 +41,7 @@ static int mixart_wait_nice_for_register_value(struct mixart_mgr *mgr,
do { /* we may take too long time in this loop.
* so give controls back to kernel if needed.
*/
- cond_resched();
+ cond_resched_stall();

read = readl_be( MIXART_MEM( mgr, offset ));
if(is_egal) {
diff --git a/sound/pci/pcxhr/pcxhr_core.c b/sound/pci/pcxhr/pcxhr_core.c
index 23f253effb4f..221eb6570c5e 100644
--- a/sound/pci/pcxhr/pcxhr_core.c
+++ b/sound/pci/pcxhr/pcxhr_core.c
@@ -304,8 +304,6 @@ int pcxhr_load_xilinx_binary(struct pcxhr_mgr *mgr,
PCXHR_OUTPL(mgr, PCXHR_PLX_CHIPSC, chipsc);
mask >>= 1;
}
- /* don't take too much time in this loop... */
- cond_resched();
}
chipsc &= ~(PCXHR_CHIPSC_DATA_CLK | PCXHR_CHIPSC_DATA_IN);
PCXHR_OUTPL(mgr, PCXHR_PLX_CHIPSC, chipsc);
@@ -356,9 +354,6 @@ static int pcxhr_download_dsp(struct pcxhr_mgr *mgr, const struct firmware *dsp)
PCXHR_OUTPB(mgr, PCXHR_DSP_TXH, data[0]);
PCXHR_OUTPB(mgr, PCXHR_DSP_TXM, data[1]);
PCXHR_OUTPB(mgr, PCXHR_DSP_TXL, data[2]);
-
- /* don't take too much time in this loop... */
- cond_resched();
}
/* give some time to boot the DSP */
msleep(PCXHR_WAIT_DEFAULT);
diff --git a/sound/pci/vx222/vx222_ops.c b/sound/pci/vx222/vx222_ops.c
index 3e7e928b24f8..84a59566b036 100644
--- a/sound/pci/vx222/vx222_ops.c
+++ b/sound/pci/vx222/vx222_ops.c
@@ -376,8 +376,6 @@ static int vx2_load_xilinx_binary(struct vx_core *chip, const struct firmware *x
for (i = 0; i < xilinx->size; i++, image++) {
if (put_xilinx_data(chip, port, 8, *image) < 0)
return -EINVAL;
- /* don't take too much time in this loop... */
- cond_resched();
}
put_xilinx_data(chip, port, 4, 0xff); /* end signature */

diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c
index ab95fb34a635..e734d2f5f711 100644
--- a/sound/x86/intel_hdmi_audio.c
+++ b/sound/x86/intel_hdmi_audio.c
@@ -1020,7 +1020,6 @@ static void wait_clear_underrun_bit(struct snd_intelhad *intelhaddata)
if (!(val & AUD_HDMI_STATUS_MASK_UNDERRUN))
return;
udelay(100);
- cond_resched();
had_write_register(intelhaddata, AUD_HDMI_STATUS, val);
}
dev_err(intelhaddata->dev, "Unable to clear UNDERRUN bits\n");
--
2.31.1