public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: mmotm 2008-11-13-17-22 uploaded (RTC build error)
       [not found] <200811140122.mAE1MeAG015836@imap1.linux-foundation.org>
@ 2008-11-14  5:16 ` Randy Dunlap
  2008-11-14  6:54   ` Alessandro Zummo
  2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (dquot build errors) Randy Dunlap
  2008-11-14  5:18 ` mmotm 2008-11-13-17-22 uploaded (pc-speaker) Randy Dunlap
  2 siblings, 1 reply; 13+ messages in thread
From: Randy Dunlap @ 2008-11-14  5:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: a.zummo

  CC [M]  drivers/rtc/rtc-dev.o
mmotm-2008-1113-1722/drivers/rtc/rtc-dev.c: In function 'rtc_dev_prepare':
mmotm-2008-1113-1722/drivers/rtc/rtc-dev.c:505: error: assignment of read-only location

-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (dquot build errors)
       [not found] <200811140122.mAE1MeAG015836@imap1.linux-foundation.org>
  2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (RTC build error) Randy Dunlap
@ 2008-11-14  5:16 ` Randy Dunlap
  2008-11-14  5:18 ` mmotm 2008-11-13-17-22 uploaded (pc-speaker) Randy Dunlap
  2 siblings, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2008-11-14  5:16 UTC (permalink / raw)
  To: linux-kernel; +Cc: jack

mmotm-2008-1113-1722/fs/dquot.c:1391: error: redefinition of '__kstrtab_dquot_reserve_space'
mmotm-2008-1113-1722/fs/dquot.c:1352: error: previous definition of '__kstrtab_dquot_reserve_space' was here
mmotm-2008-1113-1722/fs/dquot.c:1391: error: redefinition of '__ksymtab_dquot_reserve_space'
mmotm-2008-1113-1722/fs/dquot.c:1352: error: previous definition of '__ksymtab_dquot_reserve_space' was here
mmotm-2008-1113-1722/fs/dquot.c:2101: error: redefinition of '__kstrtab_vfs_dq_quota_on_remount'
mmotm-2008-1113-1722/fs/dquot.c:1974: error: previous definition of '__kstrtab_vfs_dq_quota_on_remount' was here
mmotm-2008-1113-1722/fs/dquot.c:2101: error: redefinition of '__ksymtab_vfs_dq_quota_on_remount'
mmotm-2008-1113-1722/fs/dquot.c:1974: error: previous definition of '__ksymtab_vfs_dq_quota_on_remount' was here

-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
       [not found] <200811140122.mAE1MeAG015836@imap1.linux-foundation.org>
  2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (RTC build error) Randy Dunlap
  2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (dquot build errors) Randy Dunlap
@ 2008-11-14  5:18 ` Randy Dunlap
  2008-11-14  6:36   ` Takashi Iwai
  2 siblings, 1 reply; 13+ messages in thread
From: Randy Dunlap @ 2008-11-14  5:18 UTC (permalink / raw)
  To: linux-kernel; +Cc: tiwai


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

-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  5:18 ` mmotm 2008-11-13-17-22 uploaded (pc-speaker) Randy Dunlap
@ 2008-11-14  6:36   ` Takashi Iwai
  2008-11-14  6:47     ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-14  6:36 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-kernel

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.


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  6:36   ` Takashi Iwai
@ 2008-11-14  6:47     ` Takashi Iwai
  2008-11-14  8:03       ` Peter Zijlstra
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-14  6:47 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Peter Zijlstra, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
  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 <a.p.zijlstra@chello.nl>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

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?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (RTC build error)
  2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (RTC build error) Randy Dunlap
@ 2008-11-14  6:54   ` Alessandro Zummo
  0 siblings, 0 replies; 13+ messages in thread
From: Alessandro Zummo @ 2008-11-14  6:54 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-kernel, a.zummo

On Thu, 13 Nov 2008 21:16:01 -0800
Randy Dunlap <randy.dunlap@oracle.com> wrote:

>   CC [M]  drivers/rtc/rtc-dev.o
> mmotm-2008-1113-1722/drivers/rtc/rtc-dev.c: In function 'rtc_dev_prepare':
> mmotm-2008-1113-1722/drivers/rtc/rtc-dev.c:505: error: assignment of read-only location

 thanks, I already sent an updated version to Andrew.

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  6:47     ` Takashi Iwai
@ 2008-11-14  8:03       ` Peter Zijlstra
  2008-11-14  8:17         ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2008-11-14  8:03 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
>   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 <a.p.zijlstra@chello.nl>
>     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> 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?


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  8:03       ` Peter Zijlstra
@ 2008-11-14  8:17         ` Takashi Iwai
  2008-11-14  8:30           ` Peter Zijlstra
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-14  8:17 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> >   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 <a.p.zijlstra@chello.nl>
> >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > 
> > 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.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  8:17         ` Takashi Iwai
@ 2008-11-14  8:30           ` Peter Zijlstra
  2008-11-14  8:36             ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2008-11-14  8:30 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> > >   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 <a.p.zijlstra@chello.nl>
> > >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > 
> > > 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, could you, for now, try to use HRTIMER_CB_IRQSAFE_UNLOCKED ?




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  8:30           ` Peter Zijlstra
@ 2008-11-14  8:36             ` Takashi Iwai
  2008-11-14 16:04               ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-14  8:36 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> > > >   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 <a.p.zijlstra@chello.nl>
> > > >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > > 
> > > > 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.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14  8:36             ` Takashi Iwai
@ 2008-11-14 16:04               ` Takashi Iwai
  2008-11-26 10:52                 ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-14 16:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> > > > >   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 <a.p.zijlstra@chello.nl>
> > > > >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > > > 
> > > > > 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.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-14 16:04               ` Takashi Iwai
@ 2008-11-26 10:52                 ` Takashi Iwai
  2008-11-26 13:40                   ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2008-11-26 10:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> > > > > >   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 <a.p.zijlstra@chello.nl>
> > > > > >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > > > > 
> > > > > > 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...


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: mmotm 2008-11-13-17-22 uploaded (pc-speaker)
  2008-11-26 10:52                 ` Takashi Iwai
@ 2008-11-26 13:40                   ` Takashi Iwai
  0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2008-11-26 13:40 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Linus Torvalds, Ingo Molnar, Randy Dunlap, Stephen Rothwell,
	Andrew Morton, linux-kernel

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 <a.p.zijlstra@chello.nl>
> > > > > > >   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 <a.p.zijlstra@chello.nl>
> > > > > > >     Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > > > > > 
> > > > > > > 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2008-11-26 13:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <200811140122.mAE1MeAG015836@imap1.linux-foundation.org>
2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (RTC build error) Randy Dunlap
2008-11-14  6:54   ` Alessandro Zummo
2008-11-14  5:16 ` mmotm 2008-11-13-17-22 uploaded (dquot build errors) Randy Dunlap
2008-11-14  5:18 ` mmotm 2008-11-13-17-22 uploaded (pc-speaker) Randy Dunlap
2008-11-14  6:36   ` Takashi Iwai
2008-11-14  6:47     ` Takashi Iwai
2008-11-14  8:03       ` Peter Zijlstra
2008-11-14  8:17         ` Takashi Iwai
2008-11-14  8:30           ` Peter Zijlstra
2008-11-14  8:36             ` Takashi Iwai
2008-11-14 16:04               ` Takashi Iwai
2008-11-26 10:52                 ` Takashi Iwai
2008-11-26 13:40                   ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox