From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753915AbYKZNkV (ORCPT ); Wed, 26 Nov 2008 08:40:21 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752567AbYKZNkJ (ORCPT ); Wed, 26 Nov 2008 08:40:09 -0500 Received: from ns2.suse.de ([195.135.220.15]:35616 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752564AbYKZNkH (ORCPT ); Wed, 26 Nov 2008 08:40:07 -0500 Date: Wed, 26 Nov 2008 14:40:05 +0100 Message-ID: From: Takashi Iwai To: Peter Zijlstra Cc: Linus Torvalds , Ingo Molnar , Randy Dunlap , Stephen Rothwell , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker) In-Reply-To: References: <200811140122.mAE1MeAG015836@imap1.linux-foundation.org> <491D0A32.1020401@oracle.com> <1226649794.7685.6920.camel@twins> <1226651401.7685.6977.camel@twins> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 Emacs/22.3 (x86_64-suse-linux-gnu) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Wed, 26 Nov 2008 11:52:12 +0100, I wrote: > > At Fri, 14 Nov 2008 17:04:46 +0100, > I wrote: > > > > At Fri, 14 Nov 2008 09:36:10 +0100, > > I wrote: > > > > > > At Fri, 14 Nov 2008 09:30:01 +0100, > > > Peter Zijlstra wrote: > > > > > > > > On Fri, 2008-11-14 at 09:17 +0100, Takashi Iwai wrote: > > > > > At Fri, 14 Nov 2008 09:03:14 +0100, > > > > > Peter Zijlstra wrote: > > > > > > > > > > > > On Fri, 2008-11-14 at 07:47 +0100, Takashi Iwai wrote: > > > > > > > At Fri, 14 Nov 2008 07:36:54 +0100, > > > > > > > I wrote: > > > > > > > > > > > > > > > > At Thu, 13 Nov 2008 21:18:42 -0800, > > > > > > > > Randy Dunlap wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > mmotm-2008-1113-1722/sound/drivers/pcsp/pcsp.c: In function 'snd_card_pcsp_probe': > > > > > > > > > mmotm-2008-1113-1722/sound/drivers/pcsp/pcsp.c:99: error: 'HRTIMER_CB_IRQSAFE' undeclared (first use in this function) > > > > > > > > > > > > > > > > > > # CONFIG_SND_HRTIMER is not set > > > > > > > > > > > > > > > > snd-pcsp and CONFIG_SND_HRTIMER are independent. > > > > > > > > The snd-pcsp driver code isn't changed over weeks, thus it must be the > > > > > > > > change in hrtimer side. > > > > > > > > > > > > > > It's turned out to be the recent commint in the upstream: > > > > > > > > > > > > > > commit 621a0d5207c18012cb39932f2d9830a11a6cb03d > > > > > > > Author: Peter Zijlstra > > > > > > > Date: Wed Nov 12 09:36:35 2008 +0100 > > > > > > > > > > > > > > hrtimer: clean up unused callback modes > > > > > > > > > > > > > > Impact: cleanup > > > > > > > > > > > > > > git grep HRTIMER_CB_IRQSAFE revealed half the callback modes are actually > > > > > > > unused. > > > > > > > > > > > > > > Signed-off-by: Peter Zijlstra > > > > > > > Signed-off-by: Ingo Molnar > > > > > > > > > > > > > > This hits on snd-pcsp driver on linux-next, since it was switched to > > > > > > > use this dropped flag. Now we get a build error. > > > > > > > > > > > > > > Can this commit be reverted? > > > > > > > > > > > > I think we determined the silly pc speaker driver should be using the > > > > > > SOFTIRQ timer, why was this changed back again? > > > > > > > > > > It uses a tasklet inside now. > > > > > The background story is: pcsp driver does register bit flips at each > > > > > hrtimer callback. This should be done as accurate as possible for the > > > > > sound quality (heh, who matters?). The register flip itself doesn't > > > > > take time and no lock problem. Thus, IRQSAFE is more appropriate just > > > > > for this task. > > > > > > > > > > The reason we used the softirq mode is the call of the ALSA core > > > > > update part. This is eventually called after the given samples have > > > > > been processed. And, this could cause a spin deadlock if called > > > > > directly from hrtimer callback. > > > > > > > > > > In the latest code, the call of ALSA PCM core is off-loaded via > > > > > tasklet for avoiding both spin deadlock and too long hrtimer > > > > > handling. > > > > > > > > Aside from the fact that I think tasklets should die a horrible death > > > > too, > > > > > > Oh, if you'll kill them, please provide something compatible... > > > > > > > could you, for now, try to use HRTIMER_CB_IRQSAFE_UNLOCKED ? > > > > > > OK, I'll check it later. > > > > Looks running, so far. I fixed it on for-next branch. > > Just checking again, and found out that this is broken. > > The behavior of HRTIMER_CB_IRQSAFE_UNLOCKED isn't compatible with > HRTIMER_CB_IRQSAFE in the pretty fundamental manner for restarting. > So, just replacing with IRQSAFE_UNLOCKED doesn't work... Meanwhile, I fixed the pcsp driver side so that it works now with HRTIMER_CB_IRQSAFE_UNLOCKED mode. The problem was that pcsp driver expected the callback function gets called immediately when started with zero expire time. It worked with HRTIMER_CB_SOFT and _IRQSAFE, but not with others. Takashi