* [PATCH v2] ALSA: firewire: Replace tasklet with work
@ 2020-09-09 16:36 Takashi Iwai
2020-09-10 11:43 ` Takashi Sakamoto
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2020-09-09 16:36 UTC (permalink / raw)
To: alsa-devel
The tasklet is an old API that should be deprecated, usually can be
converted to another decent API. In FireWire driver, a tasklet is
still used for offloading the AMDTP PCM stream handling. It can be
achieved gracefully with a work queued, too.
This patch replaces the tasklet usage in firewire-lib driver with a
simple work. The conversion is fairly straightforward but for the
in_interrupt() checks that are replaced with the check using the
current_work().
Note that in_interrupt() in amdtp_packet tracepoint is still kept as
is. This is the place that is probed by both softirq of 1394 OHCI and
a user task of a PCM application, and the work handling is already
filtered in amdtp_domain_stream_pcm_pointer().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: Drop in_interrupt() conversion in tracepoint
sound/firewire/amdtp-stream.c | 25 +++++++++++++------------
sound/firewire/amdtp-stream.h | 2 +-
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index ee1c428b1fd3..4e2f2bb7879f 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -64,7 +64,7 @@
#define IT_PKT_HEADER_SIZE_CIP 8 // For 2 CIP header.
#define IT_PKT_HEADER_SIZE_NO_CIP 0 // Nothing.
-static void pcm_period_tasklet(struct tasklet_struct *t);
+static void pcm_period_work(struct work_struct *work);
/**
* amdtp_stream_init - initialize an AMDTP stream structure
@@ -94,7 +94,7 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
s->flags = flags;
s->context = ERR_PTR(-1);
mutex_init(&s->mutex);
- tasklet_setup(&s->period_tasklet, pcm_period_tasklet);
+ INIT_WORK(&s->period_work, pcm_period_work);
s->packet_index = 0;
init_waitqueue_head(&s->callback_wait);
@@ -203,7 +203,7 @@ int amdtp_stream_add_pcm_hw_constraints(struct amdtp_stream *s,
// Linux driver for 1394 OHCI controller voluntarily flushes isoc
// context when total size of accumulated context header reaches
- // PAGE_SIZE. This kicks tasklet for the isoc context and brings
+ // PAGE_SIZE. This kicks work for the isoc context and brings
// callback in the middle of scheduled interrupts.
// Although AMDTP streams in the same domain use the same events per
// IRQ, use the largest size of context header between IT/IR contexts.
@@ -333,7 +333,7 @@ EXPORT_SYMBOL(amdtp_stream_get_max_payload);
*/
void amdtp_stream_pcm_prepare(struct amdtp_stream *s)
{
- tasklet_kill(&s->period_tasklet);
+ cancel_work_sync(&s->period_work);
s->pcm_buffer_pointer = 0;
s->pcm_period_pointer = 0;
}
@@ -437,13 +437,14 @@ static void update_pcm_pointers(struct amdtp_stream *s,
s->pcm_period_pointer += frames;
if (s->pcm_period_pointer >= pcm->runtime->period_size) {
s->pcm_period_pointer -= pcm->runtime->period_size;
- tasklet_hi_schedule(&s->period_tasklet);
+ queue_work(system_highpri_wq, &s->period_work);
}
}
-static void pcm_period_tasklet(struct tasklet_struct *t)
+static void pcm_period_work(struct work_struct *work)
{
- struct amdtp_stream *s = from_tasklet(s, t, period_tasklet);
+ struct amdtp_stream *s = container_of(work, struct amdtp_stream,
+ period_work);
struct snd_pcm_substream *pcm = READ_ONCE(s->pcm);
if (pcm)
@@ -794,7 +795,7 @@ static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs,
static inline void cancel_stream(struct amdtp_stream *s)
{
s->packet_index = -1;
- if (in_interrupt())
+ if (current_work() == &s->period_work)
amdtp_stream_pcm_abort(s);
WRITE_ONCE(s->pcm_buffer_pointer, SNDRV_PCM_POS_XRUN);
}
@@ -1184,7 +1185,7 @@ unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d,
if (irq_target && amdtp_stream_running(irq_target)) {
// This function is called in software IRQ context of
- // period_tasklet or process context.
+ // period_work or process context.
//
// When the software IRQ context was scheduled by software IRQ
// context of IT contexts, queued packets were already handled.
@@ -1195,9 +1196,9 @@ unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d,
// immediately to keep better granularity of PCM pointer.
//
// Later, the process context will sometimes schedules software
- // IRQ context of the period_tasklet. Then, no need to flush the
+ // IRQ context of the period_work. Then, no need to flush the
// queue by the same reason as described in the above
- if (!in_interrupt()) {
+ if (current_work() != &s->period_work) {
// Queued packet should be processed without any kernel
// preemption to keep latency against bus cycle.
preempt_disable();
@@ -1263,7 +1264,7 @@ static void amdtp_stream_stop(struct amdtp_stream *s)
return;
}
- tasklet_kill(&s->period_tasklet);
+ cancel_work_sync(&s->period_work);
fw_iso_context_stop(s->context);
fw_iso_context_destroy(s->context);
s->context = ERR_PTR(-1);
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index 703b710aaf7f..2ceb57d1d58e 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -163,7 +163,7 @@ struct amdtp_stream {
/* For a PCM substream processing. */
struct snd_pcm_substream *pcm;
- struct tasklet_struct period_tasklet;
+ struct work_struct period_work;
snd_pcm_uframes_t pcm_buffer_pointer;
unsigned int pcm_period_pointer;
--
2.16.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ALSA: firewire: Replace tasklet with work
2020-09-09 16:36 [PATCH v2] ALSA: firewire: Replace tasklet with work Takashi Iwai
@ 2020-09-10 11:43 ` Takashi Sakamoto
2020-09-11 16:23 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Sakamoto @ 2020-09-10 11:43 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
Hi,
On Wed, Sep 09, 2020 at 06:36:59PM +0200, Takashi Iwai wrote:
> The tasklet is an old API that should be deprecated, usually can be
> converted to another decent API. In FireWire driver, a tasklet is
> still used for offloading the AMDTP PCM stream handling. It can be
> achieved gracefully with a work queued, too.
>
> This patch replaces the tasklet usage in firewire-lib driver with a
> simple work. The conversion is fairly straightforward but for the
> in_interrupt() checks that are replaced with the check using the
> current_work().
>
> Note that in_interrupt() in amdtp_packet tracepoint is still kept as
> is. This is the place that is probed by both softirq of 1394 OHCI and
> a user task of a PCM application, and the work handling is already
> filtered in amdtp_domain_stream_pcm_pointer().
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> v1->v2: Drop in_interrupt() conversion in tracepoint
>
> sound/firewire/amdtp-stream.c | 25 +++++++++++++------------
> sound/firewire/amdtp-stream.h | 2 +-
> 2 files changed, 14 insertions(+), 13 deletions(-)
The v2 patch looks good to me.
Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Thanks for your work!
Regards
Takashi Sakamoto
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ALSA: firewire: Replace tasklet with work
2020-09-10 11:43 ` Takashi Sakamoto
@ 2020-09-11 16:23 ` Takashi Iwai
0 siblings, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2020-09-11 16:23 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel
On Thu, 10 Sep 2020 13:43:59 +0200,
Takashi Sakamoto wrote:
>
> Hi,
>
> On Wed, Sep 09, 2020 at 06:36:59PM +0200, Takashi Iwai wrote:
> > The tasklet is an old API that should be deprecated, usually can be
> > converted to another decent API. In FireWire driver, a tasklet is
> > still used for offloading the AMDTP PCM stream handling. It can be
> > achieved gracefully with a work queued, too.
> >
> > This patch replaces the tasklet usage in firewire-lib driver with a
> > simple work. The conversion is fairly straightforward but for the
> > in_interrupt() checks that are replaced with the check using the
> > current_work().
> >
> > Note that in_interrupt() in amdtp_packet tracepoint is still kept as
> > is. This is the place that is probed by both softirq of 1394 OHCI and
> > a user task of a PCM application, and the work handling is already
> > filtered in amdtp_domain_stream_pcm_pointer().
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> > v1->v2: Drop in_interrupt() conversion in tracepoint
> >
> > sound/firewire/amdtp-stream.c | 25 +++++++++++++------------
> > sound/firewire/amdtp-stream.h | 2 +-
> > 2 files changed, 14 insertions(+), 13 deletions(-)
>
> The v2 patch looks good to me.
>
> Tested-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> Acked-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>
> Thanks for your work!
Thanks for a quick review. Now I applied it.
Takashi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-11 16:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 16:36 [PATCH v2] ALSA: firewire: Replace tasklet with work Takashi Iwai
2020-09-10 11:43 ` Takashi Sakamoto
2020-09-11 16:23 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).