mbox series

[0/5] firewire: use sleepable workqueue to handle 1394 OHCI IT/IR context events

Message ID 20240904125155.461886-1-o-takashi@sakamocchi.jp (mailing list archive)
Headers show
Series firewire: use sleepable workqueue to handle 1394 OHCI IT/IR context events | expand

Message

Takashi Sakamoto Sept. 4, 2024, 12:51 p.m. UTC
Hi,

This series of changes updates my previous RFT[1] to apply for v6.12
kernel. For the detail, please refer to the previous one.

To Iwai-san, this series includes the change for sound subsystem as
well. All of changes are specific to ALSA firewire stack, so I would
like to send it to Linus as the part of firewire subsystem updates if
you do not mind it.

Changes from the RFT:
* WQ_FREEZABLE is newly supported in the workqueue
* Improve code comment in IEC 61883-1/6 packet streaming engine
* Avoid dead lock in the calls of workqueue sync API

[1] https://lore.kernel.org/lkml/20240901110642.154523-1-o-takashi@sakamocchi.jp/


Regards

Takashi Sakamoto (5):
  firewire: core: allocate workqueue to handle isochronous contexts in
    card
  firewire: core: add local API to queue work item to workqueue specific
    to isochronous contexts
  firewire: ohci: operate IT/IR events in sleepable work process instead
    of tasklet softIRQ
  firewire: core: non-atomic memory allocation for isochronous event to
    user client
  ALSA: firewire: use nonatomic PCM operation

 drivers/firewire/core-card.c             | 33 ++++++++++++--
 drivers/firewire/core-cdev.c             |  4 +-
 drivers/firewire/core-iso.c              | 30 ++++++++++++-
 drivers/firewire/core.h                  | 14 +++++-
 drivers/firewire/ohci.c                  | 57 +++++++++++++++++++-----
 include/linux/firewire.h                 |  3 ++
 sound/firewire/amdtp-stream.c            | 34 +++++++++++---
 sound/firewire/bebob/bebob_pcm.c         |  1 +
 sound/firewire/dice/dice-pcm.c           |  1 +
 sound/firewire/digi00x/digi00x-pcm.c     |  1 +
 sound/firewire/fireface/ff-pcm.c         |  1 +
 sound/firewire/fireworks/fireworks_pcm.c |  1 +
 sound/firewire/isight.c                  |  1 +
 sound/firewire/motu/motu-pcm.c           |  1 +
 sound/firewire/oxfw/oxfw-pcm.c           |  1 +
 sound/firewire/tascam/tascam-pcm.c       |  1 +
 16 files changed, 157 insertions(+), 27 deletions(-)

Comments

Takashi Sakamoto Sept. 5, 2024, 8:33 a.m. UTC | #1
On Wed, Sep 04, 2024 at 09:51:49PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> This series of changes updates my previous RFT[1] to apply for v6.12
> kernel. For the detail, please refer to the previous one.
> 
> To Iwai-san, this series includes the change for sound subsystem as
> well. All of changes are specific to ALSA firewire stack, so I would
> like to send it to Linus as the part of firewire subsystem updates if
> you do not mind it.
> 
> Changes from the RFT:
> * WQ_FREEZABLE is newly supported in the workqueue
> * Improve code comment in IEC 61883-1/6 packet streaming engine
> * Avoid dead lock in the calls of workqueue sync API
> 
> [1] https://lore.kernel.org/lkml/20240901110642.154523-1-o-takashi@sakamocchi.jp/
> 
> 
> Regards
> 
> Takashi Sakamoto (5):
>   firewire: core: allocate workqueue to handle isochronous contexts in
>     card
>   firewire: core: add local API to queue work item to workqueue specific
>     to isochronous contexts
>   firewire: ohci: operate IT/IR events in sleepable work process instead
>     of tasklet softIRQ
>   firewire: core: non-atomic memory allocation for isochronous event to
>     user client
>   ALSA: firewire: use nonatomic PCM operation
> 
>  drivers/firewire/core-card.c             | 33 ++++++++++++--
>  drivers/firewire/core-cdev.c             |  4 +-
>  drivers/firewire/core-iso.c              | 30 ++++++++++++-
>  drivers/firewire/core.h                  | 14 +++++-
>  drivers/firewire/ohci.c                  | 57 +++++++++++++++++++-----
>  include/linux/firewire.h                 |  3 ++
>  sound/firewire/amdtp-stream.c            | 34 +++++++++++---
>  sound/firewire/bebob/bebob_pcm.c         |  1 +
>  sound/firewire/dice/dice-pcm.c           |  1 +
>  sound/firewire/digi00x/digi00x-pcm.c     |  1 +
>  sound/firewire/fireface/ff-pcm.c         |  1 +
>  sound/firewire/fireworks/fireworks_pcm.c |  1 +
>  sound/firewire/isight.c                  |  1 +
>  sound/firewire/motu/motu-pcm.c           |  1 +
>  sound/firewire/oxfw/oxfw-pcm.c           |  1 +
>  sound/firewire/tascam/tascam-pcm.c       |  1 +
>  16 files changed, 157 insertions(+), 27 deletions(-)

I applied all of them to for-next branch.


Regards

Takashi Sakamoto
Edmund Raile Sept. 12, 2024, 9:44 p.m. UTC | #2
Hello Sakamoto-San, I came around to testing your patch [1], after RFT.

I've had to make the following changes to patch 1/5 again for it to apply to
mainline (d1f2d51b711a3b7f1ae1b46701c769c1d580fa7f), due to missing b171e20
from 2009 and a7ecbe9 from 2022.

@@ -584,9 +601,13 @@ int fw_card_add(struct fw_card *card,
 
 	generate_config_rom(card, tmp_config_rom);
 	ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
 	if (ret == 0)
 		list_add_tail(&card->link, &card_list);
+	else
+		destroy_workqueue(isoc_wq);
+
+	card->isoc_wq = isoc_wq;

 	mutex_unlock(&card_mutex);

 	return ret;
@@ -709,7 +729,9 @@ void fw_core_remove_card(struct fw_card *card)
 {
 	struct fw_card_driver dummy_driver = dummy_driver_template;
 	unsigned long flags;
 
+	might_sleep();
+
 	card->driver->update_phy_reg(card, 4,
 				     PHY_LINK_ACTIVE | PHY_CONTENDER, 0);
 	fw_schedule_bus_reset(card, false, true);
@@ -719,6 +741,7 @@ void fw_core_remove_card(struct fw_card *card)
 	dummy_driver.free_iso_context	= card->driver->free_iso_context;
 	dummy_driver.stop_iso		= card->driver->stop_iso;
 	card->driver = &dummy_driver;
+	drain_workqueue(card->isoc_wq);
 
 	spin_lock_irqsave(&card->lock, flags);
 	fw_destroy_nodes(card);

Then everything applied fine.

This resulted in 6.11.0-rc6-1-mainline-00326-gd1f2d51b711a-dirty.

Testing it with TI XIO2213B and RME Fireface 800 so far:

Initially I had a buffer freeze after 3 hours of continuous ALSA playback
from mpv:
  mpv --audio-device=alsa/sysdefault:CARD=Fireface800 Spor-Ignition.flac
accompanied by stresstest (mprime).

It didn't freeze/crash the kernel, just the audio buffer kept repeating.
Gone after power-cycling the interface and restarting playback.

Can't say with certainty whether it's related, have been unable to replicate
the issue for the past 3 days (good sign I hope).
That's why I was holding this message back a bit.

Kind regards,
Edmund Raile.

Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
Tested-by: Edmund Raile <edmund.raile@protonmail.com>
Takashi Sakamoto Sept. 13, 2024, 9:38 a.m. UTC | #3
Hi,

On Thu, Sep 12, 2024 at 09:44:52PM +0000, Edmund Raile wrote:
> Hello Sakamoto-San, I came around to testing your patch [1], after RFT.
> 
> I've had to make the following changes to patch 1/5 again for it to apply to
> mainline (d1f2d51b711a3b7f1ae1b46701c769c1d580fa7f), due to missing b171e20
> from 2009 and a7ecbe9 from 2022.
> 
> @@ -584,9 +601,13 @@ int fw_card_add(struct fw_card *card,
>  
>  	generate_config_rom(card, tmp_config_rom);
>  	ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
>  	if (ret == 0)
>  		list_add_tail(&card->link, &card_list);
> +	else
> +		destroy_workqueue(isoc_wq);
> +
> +	card->isoc_wq = isoc_wq;
> 
>  	mutex_unlock(&card_mutex);
> 
>  	return ret;
> @@ -709,7 +729,9 @@ void fw_core_remove_card(struct fw_card *card)
>  {
>  	struct fw_card_driver dummy_driver = dummy_driver_template;
>  	unsigned long flags;
>  
> +	might_sleep();
> +
>  	card->driver->update_phy_reg(card, 4,
>  				     PHY_LINK_ACTIVE | PHY_CONTENDER, 0);
>  	fw_schedule_bus_reset(card, false, true);
> @@ -719,6 +741,7 @@ void fw_core_remove_card(struct fw_card *card)
>  	dummy_driver.free_iso_context	= card->driver->free_iso_context;
>  	dummy_driver.stop_iso		= card->driver->stop_iso;
>  	card->driver = &dummy_driver;
> +	drain_workqueue(card->isoc_wq);
>  
>  	spin_lock_irqsave(&card->lock, flags);
>  	fw_destroy_nodes(card);
> 
> Then everything applied fine.
> 
> This resulted in 6.11.0-rc6-1-mainline-00326-gd1f2d51b711a-dirty.
> 
> Testing it with TI XIO2213B and RME Fireface 800 so far:
> 
> Initially I had a buffer freeze after 3 hours of continuous ALSA playback
> from mpv:
>   mpv --audio-device=alsa/sysdefault:CARD=Fireface800 Spor-Ignition.flac
> accompanied by stresstest (mprime).
> 
> It didn't freeze/crash the kernel, just the audio buffer kept repeating.
> Gone after power-cycling the interface and restarting playback.
> 
> Can't say with certainty whether it's related, have been unable to replicate
> the issue for the past 3 days (good sign I hope).
> That's why I was holding this message back a bit.
> 
> Kind regards,
> Edmund Raile.
> 
> Signed-off-by: Edmund Raile <edmund.raile@protonmail.com>
> Tested-by: Edmund Raile <edmund.raile@protonmail.com>
 
Thank you for your test. I've picked up your Tested-by tag to the
series.


Thanks

Takashi Sakamoto