linux-kernel - Re: [alsa-devel] [PATCH] Improve behaviour of Dreamcast aica ALSA driver in poor resource conditions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5h6450a6wo.wl%tiwai@suse.de>
Date:	Wed, 04 Jul 2007 10:51:19 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	"Adrian McMenamin" <adrianmcmenamin@...il.com>
Cc:	alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH] Improve behaviour of Dreamcast aica ALSA	driver in poor resource conditions

At Wed, 4 Jul 2007 00:08:46 +0100,
Adrian McMenamin wrote:
> 
> On 03/07/07, Adrian McMenamin <adrianmcmenamin@...il.com> wrote:
> > This patch stops the driver from crashing in certain situations (eg if
> > the network fails when NFS mounted), please apply.
> >
> 
> Actually, looked at this again and while the previous patch stopped
> the driver from crashing it still locked the sound subsystem. This was
> because INIT_WORK was being called on the same work_struct when the
> device recovered from the network failure. The patch below ensures
> that this does not happen and that instead, PREPARE_WORK is called.
> 
> Submitted by: Adrian McMenamin <adrian@...en.demon.co.uk>
> Signed-off by: Adrian McMenamin <adrian@...en.demon.co.uk>
> 
> --- alsa-kernel/sh/aica.c	2007-06-23 15:25:55.000000000 +0100
> +++ linux-2.6.21/sound/sh/aica.c	2007-07-04 00:01:23.000000000 +0100
> @@ -68,6 +68,7 @@
>  static struct spu_work_holder {
>  	struct work_struct spu_dma_work;
>  	void *sspointer;
> +	int already_started;
>  } spu_working;
> 
>  static struct workqueue_struct *aica_queue;
> @@ -319,7 +320,13 @@
>  	dreamcastcard = substream->pcm->private_data;
>  	/*  Use queue to do the heavy lifting */
>  	spu_working.sspointer = substream;
> -	INIT_WORK(&(spu_working.spu_dma_work), run_spu_dma);
> +	if (spu_working.already_started == 1) {
> +		PREPARE_WORK(&(spu_working.spu_dma_work), run_spu_dma);
> +	}
> +	else {
> +		INIT_WORK(&(spu_working.spu_dma_work), run_spu_dma);
> +		spu_working.already_started = 1;
> +	}
>  	queue_work(aica_queue, &(spu_working.spu_dma_work));
>  	/* Timer may already be running */
>  	if (unlikely(dreamcastcard->timer.data)) {

Better to initialize at the beginning of the driver initialization.
With this patch, the work struct is reset at each time you open the
PCM, which is inefficient and can be even racy.

Also, it'd be cleaner to move struct spu_work_holder into struct
snd_card_aica.  We'll have snd_card_aica pointer via container_of, so
sspointer can be removed (snd_card_aica has substream).


thanks,

Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/