Re: Linux 2.6.16-rc3

Re: Linux 2.6.16-rc3

[Andrew Morton]

We still have some serious bugs, several of which are in 2.6.15 as well:

- The scsi_cmd leak, which I don't think is fixed.

- The some-x86_64-boxes-use-GFP_DMA-from-bio-layer bug, which causes
  oom-killings.

- The skbuff_head_cache leak, which has been around since at least
  2.6.11.  Another box-killer, but is seems very hard to hit. 
  (mki@mozone.net, "the dreaded oom-killer (reproducable in 2.6.11 -
  2.6.16-rc1) :(")

- http://bugzilla.kernel.org/show_bug.cgi?id=6060: an apparent ACPI
  regression.

- Nathan's "sysfs-related oops during module unload", which Greg seems to
  have under control.

- http://bugzilla.kernel.org/show_bug.cgi?id=6049 - another acpi
  regression.  We have the actual offending commit here.

- A couple of random tty-related oopses reported by Jesper Juhl.  We
  don't know why these happened - they appear to not be related to the tty
  buffering changes.

- http://bugzilla.kernel.org/show_bug.cgi?id=6038, another box-killing
  acpi regression.

- Various reports similar to
  http://bugzilla.kernel.org/show_bug.cgi?id=6011, seemingly related to USB
  PCI quirk handling.

- "Ben Castricum" <lk@bencastricum.nl> reports that ppp has started
  exhibiting mysterious failures (again).

- Nasty warnings from scsi about kobject-layer things being called from
  irq context.  James has a push-it-to-process-context patch which sadly
  assumes kmalloc() is immortal, but no other fix seems to have offered
  itself.

- In http://bugzilla.kernel.org/show_bug.cgi?id=5989, Sanjoy Mahajan has
  another regression, but he's off collecting more info.

- Helge Hafting reports a usb printer regression - I don't know if that's
  still live?

- "Carlo E.  Prelz" <fluido@fluido.as> has another USB/ehci regression
  ("ATI RS480-based motherboard: stuck while booting with kernel >= 2.6.15
  rc1").

- Gerrit Bruchhuser <gbruchhaeuser@gmx.de> seems to have an aic7xxx
  regression ("AHA-7850 doesn't detect scanner anymore") but he doesn't say
  which kernel got it right.

- http://bugzilla.kernel.org/show_bug.cgi?id=5914 - a sata bug (which is
  quite unremarkable :(), but this one is reported to eat filesystems.

- Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
  regression ("alsa suspend/resume continues to fail for ens1370")

- Bjorn Nilsson <bni.swe@gmail.com> has an sk99lin regression ("3COM
  3C940, does not work anymore after upgrade to 2.6.15")

- Andrey Borzenkov <arvidjaar@mail.ru> has an acpi-cpufreq regression
  ("cannot unload acpi-cpufreq")

- "P.  Christeas" <p_christ@hol.gr> had an autofs regression ("Regression
  in Autofs, 2.6.15-git"), whic might be fixed now?

- ghrt <ghrt@dial.kappa.ro> reports an alsa regression ("PROBLEM: SB
  Live!  5.1 (emu10k1, rev.  0a) doesn't work with 2.6.15")

- jinhong hu <jinhong.hu@gmail.com> reports what appears to be a qlogic
  regression ("kernel 2.6.15 scsi problem")

- Benjamin LaHaise <bcrl@kvack.org> had an NFS problem ("NFS processes
  gettting stuck in D with currrent git").



These are clear regressions, reported in the last month by people who are
willing to test patches.  They're almost all in subsystems which have
active and professional maintainers.

Re: Linux 2.6.16-rc3

[Trond Myklebust]

On Sun, 2006-02-12 at 19:05 -0800, Andrew Morton wrote:
> We still have some serious bugs, several of which are in 2.6.15 as well:

> - Benjamin LaHaise <bcrl@kvack.org> had an NFS problem ("NFS processes
>   gettting stuck in D with currrent git").

...but which was apparently not repeatable:

        As of this afternoon's tree
        (6150c32589d1976ca8a5c987df951088c05a7542)  after the more
        recent set of nfs patches, it seems to be behaving itself.  Will
        keep sysrq enabled to see if it hits again, though.

I've had no news from Ben since then...

Cheers,
  Trond

Re: Linux 2.6.16-rc3

[Sanjoy Mahajan]

> In http://bugzilla.kernel.org/show_bug.cgi?id=5989, Sanjoy Mahajan has
> another regression, but he's off collecting more info.

I'm nearly done with bisecting (spent a day on a wild bisect goose
chase due to being careless) and I'm 95% sure the problem is
introduced by:

commit b8e4d89357fc434618a59c1047cac72641191805
Author: Bob Moore <robert.moore@intel.com>
Date:   Fri Jan 27 16:43:00 2006 -0500

    [ACPI] ACPICA 20060127

But I will know for sure shortly.

-Sanjoy

`Never underestimate the evil of which men of power are capable.'
         --Bertrand Russell, _War Crimes in Vietnam_, chapter 1.

Re: Linux 2.6.16-rc3

[Jeff Garzik]

Andrew Morton wrote:
> - http://bugzilla.kernel.org/show_bug.cgi?id=5914 - a sata bug (which is
>   quite unremarkable :(), but this one is reported to eat filesystems.

Issue closed, as the bug notes...

	Jeff

Re: Linux 2.6.16-rc3

[James Bottomley]

On Sun, 2006-02-12 at 19:05 -0800, Andrew Morton wrote:
> - The scsi_cmd leak, which I don't think is fixed.

Erm, you mean the leak caused by flush barriers?  That was verified as
fixed (albeit accidentally) in 2.6.16-rc1.

James

S3 sleep regression bisected (was Re: Linux 2.6.16-rc3)

[Sanjoy Mahajan]

> In http://bugzilla.kernel.org/show_bug.cgi?id=5989, Sanjoy Mahajan has
> another regression, but he's off collecting more info.

Now collected.  The problematic commit is:

bad: [292dd876ee765c478b27c93cc51e93a558ed58bf] Pull release into acpica branch

The longer story follows (and I'll file it with the bugzilla).  The
bug is that S3 sleep hangs on the second sleep of my TP 600X,
producing an endless loop in the dmesgs (across a serial console):

  exregion-0182 [30] ex_system_memory_space: system_memory 0 (32 width) Address=0000000023FDFFC0
  exregion-0182 [30] ex_system_memory_space: system_memory 1 (32 width) Address=0000000023FDFFC0
  exregion-0287 [30] ex_system_io_space_han: system_iO 1 (8 width) Address=00000000000000B2
  exregion-0182 [29] ex_system_memory_space: system_memory 0 (32 width) Address=0000000023FDFFC0

'git bisect' narrowed the problematic commit to:

  commit 292dd876ee765c478b27c93cc51e93a558ed58bf
  Merge: d4ec6c7cc9a15a7a529719bc3b84f46812f9842e
  9fdb62af92c741addbea15545f214a6e89460865
  Author: Len Brown <len.brown@intel.com>
  Date:   Fri Jan 27 17:18:29 2006 -0500

      Pull release into acpica branch

This commit had a slightly different bug from all the other ones I
marked as bad.  This one hung on the first S3 sleep, with the same
endless loop pattern as the other bad ones (but they hang on the
second S3 sleep).

Below is the 'git bisect log', which for fun you can put in some file
and feed to 'git bisect replay somefile' :

$ git bisect log
git-bisect start
# good: [f3bcf72eb85aba88a7bd0a6116dd0b5418590dbe] Linux v2.6.16-rc1
git-bisect good f3bcf72eb85aba88a7bd0a6116dd0b5418590dbe
# bad: [e4f9aae0d74cb7d2fd5f0eb315cf9de1118fe260] Linux v2.6.16-rc2
git-bisect bad e4f9aae0d74cb7d2fd5f0eb315cf9de1118fe260
# good: [a6df590dd8b7644c8e298e3b13442bcd6ceeb739] Merge branch 'for-linus' of master.kernel.org:/pub/scm/linux/kernel/git/roland/infiniband
git-bisect good a6df590dd8b7644c8e298e3b13442bcd6ceeb739
# good: [e0e851cf30f1a9bd2e2a7624e9810378d6a2b072] reiserfs: reiserfs hang and performance fix for data=journal mode
git-bisect good e0e851cf30f1a9bd2e2a7624e9810378d6a2b072
# bad: [59ed2f59e4ea6a32f9591e378da7935f713a7000] Merge branch 'release' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6
git-bisect bad 59ed2f59e4ea6a32f9591e378da7935f713a7000
# good: [9fdb62af92c741addbea15545f214a6e89460865] [ACPI] merge 3549 4320 4485 4588 4980 5483 5651 acpica asus fops pnpacpi branches into release
git-bisect good 9fdb62af92c741addbea15545f214a6e89460865
# good: [61ee9cd5f2e76859222c1d64394ae633f9080163] PowerPC/PCI Hotplug build break
git-bisect good 61ee9cd5f2e76859222c1d64394ae633f9080163
# good: [e6da74e1f20ea7822e52a9e4fbd3d25bd907e471] Merge with /pub/scm/linux/kernel/git/torvalds/linux-2.6.git
git-bisect good e6da74e1f20ea7822e52a9e4fbd3d25bd907e471
# bad: [b8e4d89357fc434618a59c1047cac72641191805] [ACPI] ACPICA 20060127
git-bisect bad b8e4d89357fc434618a59c1047cac72641191805
# bad: [292dd876ee765c478b27c93cc51e93a558ed58bf] Pull release into acpica branch
git-bisect bad 292dd876ee765c478b27c93cc51e93a558ed58bf
# good: [d4ec6c7cc9a15a7a529719bc3b84f46812f9842e] [ACPI] remove "Resource isn't an IRQ" warning
git-bisect good d4ec6c7cc9a15a7a529719bc3b84f46812f9842e

Re: Linux 2.6.16-rc3

[Arjan van de Ven]

On Sun, 2006-02-12 at 19:05 -0800, Andrew Morton wrote:
> We still have some serious bugs, several of which are in 2.6.15 as well:
> 
> - The scsi_cmd leak, which I don't think is fixed.

didn't this got nailed down to a 2.6.15 specific queueing bug, fixed in
2.6.16-rc ?

Re: Linux 2.6.16-rc3

[Jens Axboe]

On Sun, Feb 12 2006, Andrew Morton wrote:
> 
> We still have some serious bugs, several of which are in 2.6.15 as well:
> 
> - The scsi_cmd leak, which I don't think is fixed.

It is fixed in 2.6.16-rcX.

> - The some-x86_64-boxes-use-GFP_DMA-from-bio-layer bug, which causes
>   oom-killings.

Still pending.

-- 
Jens Axboe

Re: Linux 2.6.16-rc3

[Patrizio Bassi]

Andrew Morton ha scritto:
> We still have some serious bugs, several of which are in 2.6.15 as well:
>
> - The scsi_cmd leak, which I don't think is fixed.
>
> - The some-x86_64-boxes-use-GFP_DMA-from-bio-layer bug, which causes
>   oom-killings.
>
> - The skbuff_head_cache leak, which has been around since at least
>   2.6.11.  Another box-killer, but is seems very hard to hit. 
>   (mki@mozone.net, "the dreaded oom-killer (reproducable in 2.6.11 -
>   2.6.16-rc1) :(")
>
> - http://bugzilla.kernel.org/show_bug.cgi?id=6060: an apparent ACPI
>   regression.
>
> - Nathan's "sysfs-related oops during module unload", which Greg seems to
>   have under control.
>
> - http://bugzilla.kernel.org/show_bug.cgi?id=6049 - another acpi
>   regression.  We have the actual offending commit here.
>
> - A couple of random tty-related oopses reported by Jesper Juhl.  We
>   don't know why these happened - they appear to not be related to the tty
>   buffering changes.
>
> - http://bugzilla.kernel.org/show_bug.cgi?id=6038, another box-killing
>   acpi regression.
>
> - Various reports similar to
>   http://bugzilla.kernel.org/show_bug.cgi?id=6011, seemingly related to USB
>   PCI quirk handling.
>
> - "Ben Castricum" <lk@bencastricum.nl> reports that ppp has started
>   exhibiting mysterious failures (again).
>
> - Nasty warnings from scsi about kobject-layer things being called from
>   irq context.  James has a push-it-to-process-context patch which sadly
>   assumes kmalloc() is immortal, but no other fix seems to have offered
>   itself.
>
> - In http://bugzilla.kernel.org/show_bug.cgi?id=5989, Sanjoy Mahajan has
>   another regression, but he's off collecting more info.
>
> - Helge Hafting reports a usb printer regression - I don't know if that's
>   still live?
>
> - "Carlo E.  Prelz" <fluido@fluido.as> has another USB/ehci regression
>   ("ATI RS480-based motherboard: stuck while booting with kernel >= 2.6.15
>   rc1").
>
> - Gerrit Bruchhuser <gbruchhaeuser@gmx.de> seems to have an aic7xxx
>   regression ("AHA-7850 doesn't detect scanner anymore") but he doesn't say
>   which kernel got it right.
>
> - http://bugzilla.kernel.org/show_bug.cgi?id=5914 - a sata bug (which is
>   quite unremarkable :(), but this one is reported to eat filesystems.
>
> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
>   regression ("alsa suspend/resume continues to fail for ens1370")
>
> - Bjorn Nilsson <bni.swe@gmail.com> has an sk99lin regression ("3COM
>   3C940, does not work anymore after upgrade to 2.6.15")
>
> - Andrey Borzenkov <arvidjaar@mail.ru> has an acpi-cpufreq regression
>   ("cannot unload acpi-cpufreq")
>
> - "P.  Christeas" <p_christ@hol.gr> had an autofs regression ("Regression
>   in Autofs, 2.6.15-git"), whic might be fixed now?
>
> - ghrt <ghrt@dial.kappa.ro> reports an alsa regression ("PROBLEM: SB
>   Live!  5.1 (emu10k1, rev.  0a) doesn't work with 2.6.15")
>
> - jinhong hu <jinhong.hu@gmail.com> reports what appears to be a qlogic
>   regression ("kernel 2.6.15 scsi problem")
>
> - Benjamin LaHaise <bcrl@kvack.org> had an NFS problem ("NFS processes
>   gettting stuck in D with currrent git").
>
>
>
> These are clear regressions, reported in the last month by people who are
> willing to test patches.  They're almost all in subsystems which have
> active and professional maintainers.
>
>   
Really sad to say, but my Alsa ens1370 regression due to suspend problem
is still there.
Only fix is reboot actually. Ready to patch :)

PS.

i have a bug similar to:
http://bugzilla.kernel.org/show_bug.cgi?id=6038 (marked as blocking by
Andrew)
on my laptop.

but my dma expiry problem only happens during suspend.
I have a Sis 630 chipset with a 2.5" hitachi drive.
Been there...for ages..i can say: always...never got a working 2.6 kernel.
However in my poor opinion it's not blocking on my system, just boring.


Patrizio

Re: Linux 2.6.16-rc3

[Takashi Iwai]

At Sun, 12 Feb 2006 19:05:20 -0800,
Andrew Morton wrote:
> 
> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
>   regression ("alsa suspend/resume continues to fail for ens1370")

It's not a "regression".  PM didn't work with ens1370 at all in the
eralier version.

About the problem there, I have no idea now what's wrong.  The
suspend-to-disk works fine if the driver is built as module but not as
built-in kernel.


> - ghrt <ghrt@dial.kappa.ro> reports an alsa regression ("PROBLEM: SB
>   Live!  5.1 (emu10k1, rev.  0a) doesn't work with 2.6.15")

I couldn't find this bugzilla entry...


Takashi

Re: Linux 2.6.16-rc3

[Patrizio Bassi]

Takashi Iwai ha scritto:
> At Sun, 12 Feb 2006 19:05:20 -0800,
> Andrew Morton wrote:
>   
>> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
>>   regression ("alsa suspend/resume continues to fail for ens1370")
>>     
>
> It's not a "regression".  PM didn't work with ens1370 at all in the
> eralier version.
>
> About the problem there, I have no idea now what's wrong.  The
> suspend-to-disk works fine if the driver is built as module but not as
> built-in kernel.
>
>
>   
i wrote "regression" because before (ehm...exactly don't know...about
2.6.14 time)
after suspend i had to restart my distro's mixer values service or i
couldn't hear anything.
and...ok..it was boring but worked.

now i get 0x660 errors if i try to restore values and device seems dead,
just flood the syslog.
sound apps slow down (like 100% cpu usage) and no sound produced.

only fix is reboot.


>> - ghrt <ghrt@dial.kappa.ro> reports an alsa regression ("PROBLEM: SB
>>   Live!  5.1 (emu10k1, rev.  0a) doesn't work with 2.6.15")
>>     
>
> I couldn't find this bugzilla entry...
>
>
> Takashi
>
>   

Re: Linux 2.6.16-rc3

[Takashi Iwai]

At Mon, 13 Feb 2006 13:37:55 +0100,
Patrizio Bassi wrote:
> 
> Takashi Iwai ha scritto:
> > At Sun, 12 Feb 2006 19:05:20 -0800,
> > Andrew Morton wrote:
> >   
> >> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> >>   regression ("alsa suspend/resume continues to fail for ens1370")
> >>     
> >
> > It's not a "regression".  PM didn't work with ens1370 at all in the
> > eralier version.
> >
> > About the problem there, I have no idea now what's wrong.  The
> > suspend-to-disk works fine if the driver is built as module but not as
> > built-in kernel.
> >
> >
> >   
> i wrote "regression" because before (ehm...exactly don't know...about
> 2.6.14 time)
> after suspend i had to restart my distro's mixer values service or i
> couldn't hear anything.
> and...ok..it was boring but worked.

You abused the function which wasn't officially supported :)


Takashi

Re: Linux 2.6.16-rc3

[Patrizio Bassi]

Takashi Iwai ha scritto:
> At Mon, 13 Feb 2006 13:37:55 +0100,
> Patrizio Bassi wrote:
>   
>> Takashi Iwai ha scritto:
>>     
>>> At Sun, 12 Feb 2006 19:05:20 -0800,
>>> Andrew Morton wrote:
>>>   
>>>       
>>>> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
>>>>   regression ("alsa suspend/resume continues to fail for ens1370")
>>>>     
>>>>         
>>> It's not a "regression".  PM didn't work with ens1370 at all in the
>>> eralier version.
>>>
>>> About the problem there, I have no idea now what's wrong.  The
>>> suspend-to-disk works fine if the driver is built as module but not as
>>> built-in kernel.
>>>
>>>
>>>   
>>>       
>> i wrote "regression" because before (ehm...exactly don't know...about
>> 2.6.14 time)
>> after suspend i had to restart my distro's mixer values service or i
>> couldn't hear anything.
>> and...ok..it was boring but worked.
>>     
>
> You abused the function which wasn't officially supported :)
>
>
> Takashi
>
>   
nice i'm an abuser! :)

ok, seriously..that's bad, because before it was not implemented, so ok...
but now it fails with errors (and make apps not working properly) which
is worse.

Patrizio

Re: Linux 2.6.16-rc3

[Takashi Iwai]

At Mon, 13 Feb 2006 14:31:23 +0100,
Patrizio Bassi wrote:
> 
> Takashi Iwai ha scritto:
> > At Mon, 13 Feb 2006 13:37:55 +0100,
> > Patrizio Bassi wrote:
> >   
> >> Takashi Iwai ha scritto:
> >>     
> >>> At Sun, 12 Feb 2006 19:05:20 -0800,
> >>> Andrew Morton wrote:
> >>>   
> >>>       
> >>>> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> >>>>   regression ("alsa suspend/resume continues to fail for ens1370")
> >>>>     
> >>>>         
> >>> It's not a "regression".  PM didn't work with ens1370 at all in the
> >>> eralier version.
> >>>
> >>> About the problem there, I have no idea now what's wrong.  The
> >>> suspend-to-disk works fine if the driver is built as module but not as
> >>> built-in kernel.
> >>>
> >>>
> >>>   
> >>>       
> >> i wrote "regression" because before (ehm...exactly don't know...about
> >> 2.6.14 time)
> >> after suspend i had to restart my distro's mixer values service or i
> >> couldn't hear anything.
> >> and...ok..it was boring but worked.
> >>     
> >
> > You abused the function which wasn't officially supported :)
> >
> >
> > Takashi
> >
> >   
> nice i'm an abuser! :)
> 
> ok, seriously..that's bad, because before it was not implemented, so ok...
> but now it fails with errors (and make apps not working properly) which
> is worse.

My rough guess is the initialization order, the resume was called too
early.

What about to put sleep between snd_ensoniq_chip_init() and
snd_ak4531_resume()?  Or put more delay in snd_ak4531_resume()?


Takashi

Re: Linux 2.6.16-rc3

[Patrizio Bassi]

Takashi Iwai ha scritto:
> At Mon, 13 Feb 2006 14:31:23 +0100,
> Patrizio Bassi wrote:
>   
>> Takashi Iwai ha scritto:
>>     
>>> At Mon, 13 Feb 2006 13:37:55 +0100,
>>> Patrizio Bassi wrote:
>>>   
>>>       
>>>> Takashi Iwai ha scritto:
>>>>     
>>>>         
>>>>> At Sun, 12 Feb 2006 19:05:20 -0800,
>>>>> Andrew Morton wrote:
>>>>>   
>>>>>       
>>>>>           
>>>>>> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
>>>>>>   regression ("alsa suspend/resume continues to fail for ens1370")
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> It's not a "regression".  PM didn't work with ens1370 at all in the
>>>>> eralier version.
>>>>>
>>>>> About the problem there, I have no idea now what's wrong.  The
>>>>> suspend-to-disk works fine if the driver is built as module but not as
>>>>> built-in kernel.
>>>>>
>>>>>
>>>>>   
>>>>>       
>>>>>           
>>>> i wrote "regression" because before (ehm...exactly don't know...about
>>>> 2.6.14 time)
>>>> after suspend i had to restart my distro's mixer values service or i
>>>> couldn't hear anything.
>>>> and...ok..it was boring but worked.
>>>>     
>>>>         
>>> You abused the function which wasn't officially supported :)
>>>
>>>
>>> Takashi
>>>
>>>   
>>>       
>> nice i'm an abuser! :)
>>
>> ok, seriously..that's bad, because before it was not implemented, so ok...
>> but now it fails with errors (and make apps not working properly) which
>> is worse.
>>     
>
> My rough guess is the initialization order, the resume was called too
> early.
>
> What about to put sleep between snd_ensoniq_chip_init() and
> snd_ak4531_resume()?  Or put more delay in snd_ak4531_resume()?
>
>
> Takashi
>
>   
i'm almost sure the problem is not there (or, at least not only)
infact i get 0x660 errors (or better a long flood...) while suspending too.

there may be a bug or problem during suspending, and these problems
affect the normal resume.

however sleep is not always trustable...soif you think that's an init
problem you may add a
boolean value to check if init is completed or not and poll for TRUE
value in the resume function.

but, as i wrote before, i'm not sure the problem is **only** there.

Patrizio

Re: Linux 2.6.16-rc3

[Takashi Iwai]

At Mon, 13 Feb 2006 15:34:28 +0100,
Patrizio Bassi wrote:
> 
> Takashi Iwai ha scritto:
> > At Mon, 13 Feb 2006 14:31:23 +0100,
> > Patrizio Bassi wrote:
> >   
> >> Takashi Iwai ha scritto:
> >>     
> >>> At Mon, 13 Feb 2006 13:37:55 +0100,
> >>> Patrizio Bassi wrote:
> >>>   
> >>>       
> >>>> Takashi Iwai ha scritto:
> >>>>     
> >>>>         
> >>>>> At Sun, 12 Feb 2006 19:05:20 -0800,
> >>>>> Andrew Morton wrote:
> >>>>>   
> >>>>>       
> >>>>>           
> >>>>>> - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> >>>>>>   regression ("alsa suspend/resume continues to fail for ens1370")
> >>>>>>     
> >>>>>>         
> >>>>>>             
> >>>>> It's not a "regression".  PM didn't work with ens1370 at all in the
> >>>>> eralier version.
> >>>>>
> >>>>> About the problem there, I have no idea now what's wrong.  The
> >>>>> suspend-to-disk works fine if the driver is built as module but not as
> >>>>> built-in kernel.
> >>>>>
> >>>>>
> >>>>>   
> >>>>>       
> >>>>>           
> >>>> i wrote "regression" because before (ehm...exactly don't know...about
> >>>> 2.6.14 time)
> >>>> after suspend i had to restart my distro's mixer values service or i
> >>>> couldn't hear anything.
> >>>> and...ok..it was boring but worked.
> >>>>     
> >>>>         
> >>> You abused the function which wasn't officially supported :)
> >>>
> >>>
> >>> Takashi
> >>>
> >>>   
> >>>       
> >> nice i'm an abuser! :)
> >>
> >> ok, seriously..that's bad, because before it was not implemented, so ok...
> >> but now it fails with errors (and make apps not working properly) which
> >> is worse.
> >>     
> >
> > My rough guess is the initialization order, the resume was called too
> > early.
> >
> > What about to put sleep between snd_ensoniq_chip_init() and
> > snd_ak4531_resume()?  Or put more delay in snd_ak4531_resume()?
> >
> >
> > Takashi
> >
> >   
> i'm almost sure the problem is not there (or, at least not only)
> infact i get 0x660 errors (or better a long flood...) while suspending too.

IIRC, suspend calls resume callback once to revive the devices back.
So, basically you see the same problem here.


Takashi

Re: Linux 2.6.16-rc3

[Rafael J. Wysocki]

On Monday 13 February 2006 13:02, Takashi Iwai wrote:
> At Sun, 12 Feb 2006 19:05:20 -0800,
> Andrew Morton wrote:
> > 
> > - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> >   regression ("alsa suspend/resume continues to fail for ens1370")
> 
> It's not a "regression".  PM didn't work with ens1370 at all in the
> eralier version.
> 
> About the problem there, I have no idea now what's wrong.  The
> suspend-to-disk works fine if the driver is built as module but not as
> built-in kernel.

That may be related to the fact that modular drivers are not present in
memory during resume (just a thought).

Greetings,
Rafael

Re: Linux 2.6.16-rc3

[Takashi Iwai]

At Mon, 13 Feb 2006 14:09:51 +0100,
Rafael J. Wysocki wrote:
> 
> On Monday 13 February 2006 13:02, Takashi Iwai wrote:
> > At Sun, 12 Feb 2006 19:05:20 -0800,
> > Andrew Morton wrote:
> > > 
> > > - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> > >   regression ("alsa suspend/resume continues to fail for ens1370")
> > 
> > It's not a "regression".  PM didn't work with ens1370 at all in the
> > eralier version.
> > 
> > About the problem there, I have no idea now what's wrong.  The
> > suspend-to-disk works fine if the driver is built as module but not as
> > built-in kernel.
> 
> That may be related to the fact that modular drivers are not present in
> memory during resume (just a thought).

I think the modular drivers are on memory but the order of
re-initialization is different.


Takashi

Re: Linux 2.6.16-rc3

[Rafael J. Wysocki]

On Monday 13 February 2006 14:51, Takashi Iwai wrote:
> At Mon, 13 Feb 2006 14:09:51 +0100,
> Rafael J. Wysocki wrote:
> > 
> > On Monday 13 February 2006 13:02, Takashi Iwai wrote:
> > > At Sun, 12 Feb 2006 19:05:20 -0800,
> > > Andrew Morton wrote:
> > > > 
> > > > - Patrizio Bassi <patrizio.bassi@gmail.com> has an alsa suspend
> > > >   regression ("alsa suspend/resume continues to fail for ens1370")
> > > 
> > > It's not a "regression".  PM didn't work with ens1370 at all in the
> > > eralier version.
> > > 
> > > About the problem there, I have no idea now what's wrong.  The
> > > suspend-to-disk works fine if the driver is built as module but not as
> > > built-in kernel.
> > 
> > That may be related to the fact that modular drivers are not present in
> > memory during resume (just a thought).
> 
> I think the modular drivers are on memory but the order of
> re-initialization is different.

No, they are not (this is the part I'm sure of).  software_resume() is called
before any modules have a chance to be loaded unless you boot with
noresume, load them from an initrd and start the resume manually.

Greetings,
Rafael

Re: Linux 2.6.16-rc3

[Greg KH]

On Sun, Feb 12, 2006 at 07:05:20PM -0800, Andrew Morton wrote:
> 
> We still have some serious bugs, several of which are in 2.6.15 as well:
> 
> - Nathan's "sysfs-related oops during module unload", which Greg seems to
>   have under control.

Yes, this isn't a "regression" but has been there for a while.  It can
also only be triggered by a root user, so it's severity is much lower.

> - Various reports similar to
>   http://bugzilla.kernel.org/show_bug.cgi?id=6011, seemingly related to USB
>   PCI quirk handling.

I have a patch for this, which will be going to Linus later tonight.

> - Nasty warnings from scsi about kobject-layer things being called from
>   irq context.  James has a push-it-to-process-context patch which sadly
>   assumes kmalloc() is immortal, but no other fix seems to have offered
>   itself.

This has been the case for a long time.  I don't really think there is a
rush to get this fixed, but I really like James's proposed patch.  It's
up to him if he feels it is ready for 2.6.16 or not.

> - Helge Hafting reports a usb printer regression - I don't know if that's
>   still live?

He never reported back, and one other person who reported this, figured
out that it was bad ram in the system.  Replacing that fixed the issue.
I've also printed a lot of stuff here (tax time...) and had no problems.

> - "Carlo E.  Prelz" <fluido@fluido.as> has another USB/ehci regression
>   ("ATI RS480-based motherboard: stuck while booting with kernel >= 2.6.15
>   rc1").

Fixed by the previously mentioned EHCI quirk/handoff patch that will be
going to Linus.

So, that's it for the USB stuff, thankfully.

thanks,

greg k-h

Re: Linux 2.6.16-rc3

[James Bottomley]

On Mon, 2006-02-13 at 12:38 -0800, Greg KH wrote:
> > - Nasty warnings from scsi about kobject-layer things being called
> from
> >   irq context.  James has a push-it-to-process-context patch which
> sadly
> >   assumes kmalloc() is immortal, but no other fix seems to have
> offered
> >   itself.
> 
> This has been the case for a long time.  I don't really think there is
> a
> rush to get this fixed, but I really like James's proposed patch.
> It's
> up to him if he feels it is ready for 2.6.16 or not.

Well, I can't solve the problem that it requires memory allocation from
IRQ context to operate.  Based on that, it's an unsafe interface.  I'm
going to put it inside SCSI for 2.6.16, since it's better than what we
have now, but I don't think we can export it globally.

James

Re: Linux 2.6.16-rc3

[James Bottomley]

On Tue, 2006-02-14 at 10:34 -0600, James Bottomley wrote:
> Well, I can't solve the problem that it requires memory allocation from
> IRQ context to operate.  Based on that, it's an unsafe interface.  I'm
> going to put it inside SCSI for 2.6.16, since it's better than what we
> have now, but I don't think we can export it globally.

OK, this is what I'm proposing as the device model fix.  What it does is
thread context checking APIs throughout the device subsystem.  SCSI can
then use it simply via device_put_process_context().  Since we have to
supply the kref_work; I'd plan to do that as an additional element in
struct scsi_device.

This, by itself, won't solve the SCSI target problem, but I plan to fix
that via a device model addition which would have target alloc waiting
around for any deleted targets to disappear.

Since this is planned for post 2.6.16, we have plenty of time to argue
about it.

James

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6b355bd..4ae42de 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -338,10 +338,10 @@ struct device * get_device(struct device
  *	put_device - decrement reference count.
  *	@dev:	device in question.
  */
-void put_device(struct device * dev)
+void put_device_process_context(struct device * dev, struct kref_work *work)
 {
 	if (dev)
-		kobject_put(&dev->kobj);
+	  kobject_put_process_context(&dev->kobj, work);
 }
 
 
@@ -445,7 +445,7 @@ EXPORT_SYMBOL_GPL(device_register);
 EXPORT_SYMBOL_GPL(device_del);
 EXPORT_SYMBOL_GPL(device_unregister);
 EXPORT_SYMBOL_GPL(get_device);
-EXPORT_SYMBOL_GPL(put_device);
+EXPORT_SYMBOL_GPL(put_device_process_context);
 
 EXPORT_SYMBOL_GPL(device_create_file);
 EXPORT_SYMBOL_GPL(device_remove_file);
diff --git a/include/linux/device.h b/include/linux/device.h
index 58df18d..ac9d457 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -396,7 +396,13 @@ extern int (*platform_notify_remove)(str
  *
  */
 extern struct device * get_device(struct device * dev);
-extern void put_device(struct device * dev);
+extern void put_device_process_context(struct device * dev,
+				       struct kref_work *work);
+static inline void put_device(struct device *dev)
+{
+	put_device_process_context(dev, NULL);
+}
+						  
 
 
 /* drivers/base/power.c */
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 2a8d8da..d079fea 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -76,7 +76,12 @@ extern int kobject_register(struct kobje
 extern void kobject_unregister(struct kobject *);
 
 extern struct kobject * kobject_get(struct kobject *);
-extern void kobject_put(struct kobject *);
+extern void kobject_put_process_context(struct kobject *, struct kref_work *);
+
+static inline void kobject_put(struct kobject *kobj)
+{
+	kobject_put_process_context(kobj, NULL);
+}
 
 extern char * kobject_get_path(struct kobject *, gfp_t);
 
diff --git a/include/linux/kref.h b/include/linux/kref.h
index 6fee353..16b15db 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -18,15 +18,29 @@
 #ifdef __KERNEL__
 
 #include <linux/types.h>
+#include <linux/workqueue.h>
 #include <asm/atomic.h>
 
 struct kref {
 	atomic_t refcount;
 };
 
+struct kref_work {
+	struct work_struct work;
+	struct kref *kref;
+	void (*release)(struct kref *kref);
+};
+
 void kref_init(struct kref *kref);
 void kref_get(struct kref *kref);
-int kref_put(struct kref *kref, void (*release) (struct kref *kref));
+int kref_put_process_context(struct kref *kref,
+			     void (*release) (struct kref *kref),
+			     struct kref_work *work);
+static inline int kref_put(struct kref *kref,
+			   void (*release) (struct kref *kref))
+{
+	return kref_put_process_context(kref, release, NULL);
+}
 
 #endif /* __KERNEL__ */
 #endif /* _KREF_H_ */
diff --git a/lib/kobject.c b/lib/kobject.c
index efe67fa..6b80c54 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -372,10 +372,10 @@ static void kobject_release(struct kref 
  *
  *	Decrement the refcount, and if 0, call kobject_cleanup().
  */
-void kobject_put(struct kobject * kobj)
+void kobject_put_process_context(struct kobject * kobj, struct kref_work *work)
 {
 	if (kobj)
-		kref_put(&kobj->kref, kobject_release);
+		kref_put_process_context(&kobj->kref, kobject_release, work);
 }
 
 
@@ -537,7 +537,7 @@ EXPORT_SYMBOL(kobject_init);
 EXPORT_SYMBOL(kobject_register);
 EXPORT_SYMBOL(kobject_unregister);
 EXPORT_SYMBOL(kobject_get);
-EXPORT_SYMBOL(kobject_put);
+EXPORT_SYMBOL(kobject_put_process_context);
 EXPORT_SYMBOL(kobject_add);
 EXPORT_SYMBOL(kobject_del);
 
diff --git a/lib/kref.c b/lib/kref.c
index 0d07cc3..66231cf 100644
--- a/lib/kref.c
+++ b/lib/kref.c
@@ -13,6 +13,7 @@
 
 #include <linux/kref.h>
 #include <linux/module.h>
+#include <linux/hardirq.h>
 
 /**
  * kref_init - initialize object.
@@ -33,27 +34,47 @@ void kref_get(struct kref *kref)
 	atomic_inc(&kref->refcount);
 }
 
+static void kref_release_process_context(void *data)
+{
+	struct kref_work *work = data;
+
+	work->release(work->kref);
+}
+
 /**
- * kref_put - decrement refcount for object.
+ * kref_put_user - decrement refcount for object and put in user context
  * @kref: object.
  * @release: pointer to the function that will clean up the object when the
  *	     last reference to the object is released.
  *	     This pointer is required, and it is not acceptable to pass kfree
  *	     in as this function.
+ * @work:    pointer to a kref_work used to take the release through user
+ *	     context (may be null)
  *
- * Decrement the refcount, and if 0, call release().
+ * Decrement the refcount, and if 0, call release().  If work is not null
+ * execute release via schedule_work if not in process context.
  * Return 1 if the object was removed, otherwise return 0.  Beware, if this
  * function returns 0, you still can not count on the kref from remaining in
  * memory.  Only use the return value if you want to see if the kref is now
  * gone, not present.
  */
-int kref_put(struct kref *kref, void (*release)(struct kref *kref))
+int kref_put_process_context(struct kref *kref,
+			     void (*release)(struct kref *kref),
+			     struct kref_work *work)
 {
 	WARN_ON(release == NULL);
 	WARN_ON(release == (void (*)(struct kref *))kfree);
 
 	if (atomic_dec_and_test(&kref->refcount)) {
-		release(kref);
+		if (!work || !in_interrupt())
+			release(kref);
+		else {
+			INIT_WORK(&work->work, kref_release_process_context,
+				  work);
+			schedule_work(&work->work);
+		}
+			
+			
 		return 1;
 	}
 	return 0;
@@ -61,4 +82,4 @@ int kref_put(struct kref *kref, void (*r
 
 EXPORT_SYMBOL(kref_init);
 EXPORT_SYMBOL(kref_get);
-EXPORT_SYMBOL(kref_put);
+EXPORT_SYMBOL(kref_put_process_context);

Re: Linux 2.6.16-rc3

[Russell King]

On Wed, Feb 15, 2006 at 08:56:00PM -0500, James Bottomley wrote:
> On Tue, 2006-02-14 at 10:34 -0600, James Bottomley wrote:
> > Well, I can't solve the problem that it requires memory allocation from
> > IRQ context to operate.  Based on that, it's an unsafe interface.  I'm
> > going to put it inside SCSI for 2.6.16, since it's better than what we
> > have now, but I don't think we can export it globally.
> 
> OK, this is what I'm proposing as the device model fix.  What it does is
> thread context checking APIs throughout the device subsystem.  SCSI can
> then use it simply via device_put_process_context().  Since we have to
> supply the kref_work; I'd plan to do that as an additional element in
> struct scsi_device.
> 
> This, by itself, won't solve the SCSI target problem, but I plan to fix
> that via a device model addition which would have target alloc waiting
> around for any deleted targets to disappear.
> 
> Since this is planned for post 2.6.16, we have plenty of time to argue
> about it.

This is probably an idiotic question, but if there's something in the
scsi release handler can't be called in non-process context, why can't
scsi queue up the release processing via the work API itself, rather
than having to have this additional code and complexity for everyone?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Linux 2.6.16-rc3

[Stefan Richter]

Russell King wrote:
> On Wed, Feb 15, 2006 at 08:56:00PM -0500, James Bottomley wrote:
[...]
>>OK, this is what I'm proposing as the device model fix.  What it does is
>>thread context checking APIs throughout the device subsystem.  SCSI can
>>then use it simply via device_put_process_context().
[...]
>>Since this is planned for post 2.6.16, we have plenty of time to argue
>>about it.
> 
> This is probably an idiotic question, but if there's something in the
> scsi release handler can't be called in non-process context, why can't
> scsi queue up the release processing via the work API itself, rather
> than having to have this additional code and complexity for everyone?

Moreover, why are SCSI release handlers called in non-process context in 
the first place? IMO the fix should be to make sure that SCSI release 
handlers are always called from process context --- by the respective 
layers which manage physical devices, i.e. one or more layers beneath 
SCSI core.
-- 
Stefan Richter
-=====-=-==- --=- =----
http://arcgraph.de/sr/

Re: Linux 2.6.16-rc3

[James Bottomley]

On Thu, 2006-02-16 at 17:12 +0000, Russell King wrote:
> This is probably an idiotic question, but if there's something in the
> scsi release handler can't be called in non-process context, why can't
> scsi queue up the release processing via the work API itself, rather
> than having to have this additional code and complexity for everyone?

It's because, in order to get a guaranteed single allocation for the
workqueue to execute in user context, I need to know when the release
will be called.  The only way to do that is to add the execute in
process context directly to kref_put.

James

Re: Linux 2.6.16-rc3

[Russell King]

On Thu, Feb 16, 2006 at 09:57:32AM -0800, James Bottomley wrote:
> On Thu, 2006-02-16 at 17:12 +0000, Russell King wrote:
> > This is probably an idiotic question, but if there's something in the
> > scsi release handler can't be called in non-process context, why can't
> > scsi queue up the release processing via the work API itself, rather
> > than having to have this additional code and complexity for everyone?
> 
> It's because, in order to get a guaranteed single allocation for the
> workqueue to execute in user context, I need to know when the release
> will be called.  The only way to do that is to add the execute in
> process context directly to kref_put.

Is there something in the driver model which would prevent something
like this?

static void scsi_release_process(void *p)
{
	struct my_work *work = p;
	struct device *dev = work->dev;

	/* destroy dev */

	kfree(work);
}

static void scsi_release(struct device *dev)
{
	struct my_work *work;

	work = kmalloc(sizeof(struct my_work), GFP_ATOMIC);
	if (work) {
		INIT_WORK(&work->work, scsi_release_process, work);
		work->dev = dev;
		schedule_work(&work->work);
	} else {
		printk(KERN_ERR ...);
	}
}

where scsi_release() is the function called by the device model on the
last put of a scsi device.

I guess is more or less what you're trying to do invasively via the
driver model.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Linux 2.6.16-rc3

[James Bottomley]

On Thu, 2006-02-16 at 18:09 +0000, Russell King wrote:
> where scsi_release() is the function called by the device model on the
> last put of a scsi device.
> 
> I guess is more or less what you're trying to do invasively via the
> driver model.

Yes ... except I think more than just SCSI has the problem (and we
actually have it in more than one release function) so it seems like a
good candidate for a general abstraction.

James

Re: Linux 2.6.16-rc3

[Russell King]

On Thu, Feb 16, 2006 at 10:14:31AM -0800, James Bottomley wrote:
> On Thu, 2006-02-16 at 18:09 +0000, Russell King wrote:
> > where scsi_release() is the function called by the device model on the
> > last put of a scsi device.
> > 
> > I guess is more or less what you're trying to do invasively via the
> > driver model.
> 
> Yes ... except I think more than just SCSI has the problem (and we
> actually have it in more than one release function) so it seems like a
> good candidate for a general abstraction.

Maybe implementing it as a helper function would be the best and
simplest solution?

static void scsi_release(struct device *dev)
{
	schedule_release_process(dev, scsi_release_process);
}

where schedule_release_process() contains more or less what I posted
in the previous mailing.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: Linux 2.6.16-rc3

[James Bottomley]

On Thu, 2006-02-16 at 18:18 +0000, Russell King wrote:
> Maybe implementing it as a helper function would be the best and
> simplest solution?
> 
> static void scsi_release(struct device *dev)
> {
> 	schedule_release_process(dev, scsi_release_process);
> }
> 
> where schedule_release_process() contains more or less what I posted
> in the previous mailing.

That's almost exactly the execute_in_process_context() API that began
this discussion (and which Andi NAK'd).  However, it could possibly be
resurrected with the proviso that the caller has to feed in the
workqueue memory.  How would people feel about that?

James

Re: Linux 2.6.16-rc3

[Jens Axboe]

On Thu, Feb 16 2006, James Bottomley wrote:
> On Thu, 2006-02-16 at 18:18 +0000, Russell King wrote:
> > Maybe implementing it as a helper function would be the best and
> > simplest solution?
> > 
> > static void scsi_release(struct device *dev)
> > {
> > 	schedule_release_process(dev, scsi_release_process);
> > }
> > 
> > where schedule_release_process() contains more or less what I posted
> > in the previous mailing.
> 
> That's almost exactly the execute_in_process_context() API that began
> this discussion (and which Andi NAK'd).  However, it could possibly be
> resurrected with the proviso that the caller has to feed in the
> workqueue memory.  How would people feel about that?

That's what I suggested in the first place as well. I still think it's a
good idea, fwiw :)

-- 
Jens Axboe

Re: Linux 2.6.16-rc3

[James Bottomley]

On Thu, 2006-02-16 at 21:01 +0100, Jens Axboe wrote:
> That's what I suggested in the first place as well. I still think it's a
> good idea, fwiw :)

OK smarty pants ... some of us are a bit slower on the uptake.  How
about this then.  It won't solve the target problem, but it will solve
the device put.

James

[PATCH] add execute_in_process_context() API

We have several points in the SCSI stack (primarily for our device
functions) where we need to guarantee process context, but (given the
place where the last reference was released) we cannot guarantee this.

This API gets around the issue by executing the function directly if
the caller has process context, but scheduling a workqueue to execute
in process context if the caller doesn't have it.

Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

Index: BUILD-2.6/include/linux/workqueue.h
===================================================================
--- BUILD-2.6.orig/include/linux/workqueue.h	2006-02-17 13:02:00.000000000 -0600
+++ BUILD-2.6/include/linux/workqueue.h	2006-02-17 17:57:52.000000000 -0600
@@ -20,6 +20,12 @@
 	struct timer_list timer;
 };
 
+struct execute_work {
+	struct work_struct work;
+	void (*fn)(void *);
+	void *data;
+};
+
 #define __WORK_INITIALIZER(n, f, d) {				\
         .entry	= { &(n).entry, &(n).entry },			\
 	.func = (f),						\
@@ -74,6 +80,8 @@
 void cancel_rearming_delayed_work(struct work_struct *work);
 void cancel_rearming_delayed_workqueue(struct workqueue_struct *,
 				       struct work_struct *);
+int execute_in_process_context(void (*fn)(void *), void *,
+			       struct execute_work *);
 
 /*
  * Kill off a pending schedule_delayed_work().  Note that the work callback
Index: BUILD-2.6/kernel/workqueue.c
===================================================================
--- BUILD-2.6.orig/kernel/workqueue.c	2006-02-17 13:02:01.000000000 -0600
+++ BUILD-2.6/kernel/workqueue.c	2006-02-17 18:00:15.000000000 -0600
@@ -27,6 +27,7 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/kthread.h>
+#include <linux/hardirq.h>
 
 /*
  * The per-CPU workqueue (if single thread, we always use the first
@@ -476,6 +477,45 @@
 }
 EXPORT_SYMBOL(cancel_rearming_delayed_work);
 
+static void execute_in_process_context_work(void *data)
+{
+	void (*fn)(void *data);
+	struct execute_work *ew = data;
+
+	fn = ew->fn;
+	data = ew->data;
+
+	fn(data);
+}
+
+/**
+ * execute_in_process_context - reliably execute the routine with user context
+ * @fn:		the function to execute
+ * @data:	data to pass to the function
+ *
+ * Executes the function immediately if process context is available,
+ * otherwise schedules the function for delayed execution.
+ *
+ * Returns:	0 - function was executed
+ *		1 - function was scheduled for execution
+ */
+int execute_in_process_context(void (*fn)(void *data), void *data,
+			       struct execute_work *ew)
+{
+	if (!in_interrupt()) {
+		fn(data);
+		return 0;
+	}
+
+	INIT_WORK(&ew->work, execute_in_process_context_work, ew);
+	ew->fn = fn;
+	ew->data = data;
+	schedule_work(&ew->work);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(execute_in_process_context);
+
 int keventd_up(void)
 {
 	return keventd_wq != NULL;

Re: Linux 2.6.16-rc3

[Greg KH]

On Fri, Feb 17, 2006 at 04:42:43PM -0800, James Bottomley wrote:
> On Thu, 2006-02-16 at 21:01 +0100, Jens Axboe wrote:
> > That's what I suggested in the first place as well. I still think it's a
> > good idea, fwiw :)
> 
> OK smarty pants ... some of us are a bit slower on the uptake.  How
> about this then.  It won't solve the target problem, but it will solve
> the device put.
> 
> James
> 
> [PATCH] add execute_in_process_context() API

I like it, nice job.

Acked-by: Greg Kroah-Hartma <gregkh@suse.de>

thanks,

greg k-h

Re: Linux 2.6.16-rc3

[Roland Dreier]

 > +/**
 > + * execute_in_process_context - reliably execute the routine with user context
 > + * @fn:		the function to execute
 > + * @data:	data to pass to the function
 > + *
 > + * Executes the function immediately if process context is available,
 > + * otherwise schedules the function for delayed execution.
 > + *
 > + * Returns:	0 - function was executed
 > + *		1 - function was scheduled for execution
 > + */
 > +int execute_in_process_context(void (*fn)(void *data), void *data,
 > +			       struct execute_work *ew)
 > +{
 > +	if (!in_interrupt()) {
 > +		fn(data);
 > +		return 0;
 > +	}

Is testing in_interrupt() really sufficient to make this work?  I seem
to remember that (at least) with CONFIG_PREEMPT disabled, there are
contexts where it is not safe to sleep but where both in_interrupt()
and in_atomic() still return 0.

 - R.

Re: Linux 2.6.16-rc3

[Matthew Wilcox]

On Fri, Feb 17, 2006 at 06:12:04PM -0800, Roland Dreier wrote:
>  > +int execute_in_process_context(void (*fn)(void *data), void *data,
>  > +			       struct execute_work *ew)
>  > +{
>  > +	if (!in_interrupt()) {
>  > +		fn(data);
>  > +		return 0;
>  > +	}
> 
> Is testing in_interrupt() really sufficient to make this work?  I seem
> to remember that (at least) with CONFIG_PREEMPT disabled, there are
> contexts where it is not safe to sleep but where both in_interrupt()
> and in_atomic() still return 0.

That's correct -- in process context, with PREEMPT disabled and a spinlock
held, it's unsafe to call this function.  It works well for the SCSI
usage (where it's called either in interrupt context or in user
context with no spinlocks held), but it's not *always* safe.

I think this needs to be documented in the kerneldoc.


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

On Fri, Feb 17, 2006 at 04:42:43PM -0800, James Bottomley wrote:
> +static void execute_in_process_context_work(void *data)
> +{
> +	void (*fn)(void *data);
> +	struct execute_work *ew = data;
> +
> +	fn = ew->fn;
> +	data = ew->data;
> +
> +	fn(data);
> +}

After removing kfree(), which was here in the initial implementation,
this function became a thunk which does nothing useful - we can just
stick fn and data directly into work_struct.

> +
> +/**
> + * execute_in_process_context - reliably execute the routine with user context
> + * @fn:		the function to execute
> + * @data:	data to pass to the function
> + *
> + * Executes the function immediately if process context is available,
> + * otherwise schedules the function for delayed execution.
> + *
> + * Returns:	0 - function was executed
> + *		1 - function was scheduled for execution
> + */
> +int execute_in_process_context(void (*fn)(void *data), void *data,
> +			       struct execute_work *ew)
> +{
> +	if (!in_interrupt()) {
> +		fn(data);
> +		return 0;
> +	}
> +
> +	INIT_WORK(&ew->work, execute_in_process_context_work, ew);
> +	ew->fn = fn;
> +	ew->data = data;
> +	schedule_work(&ew->work);
> +
> +	return 1;
> +}

Then this becomes:

int execute_in_process_context(void (*fn)(void *data), void *data,
			       struct work_struct *work)
{
	if (!in_interrupt()) {
		fn(data);
		return 0;
	}

	INIT_WORK(work, fn, data);
	schedule_work(work);
	return 1;
}

(and struct execute_work is no longer needed).

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[James Bottomley]

On Sat, 2006-02-18 at 13:03 +0300, Sergey Vlasov wrote:
> After removing kfree(), which was here in the initial implementation,
> this function became a thunk which does nothing useful - we can just
> stick fn and data directly into work_struct.

Yes, thanks ... although I think there's still value to wrappering
work_struct (a bit like kref wrappers atomic_t).

James

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[James Bottomley]

On Sun, 2006-02-19 at 08:30 -0600, James Bottomley wrote:
> Yes, thanks ... although I think there's still value to wrappering
> work_struct (a bit like kref wrappers atomic_t).

OK, so how about this?

James

[PATCH] add execute_in_process_context() API

We have several points in the SCSI stack (primarily for our device
functions) where we need to guarantee process context, but (given the
place where the last reference was released) we cannot guarantee this.

This API gets around the issue by executing the function directly if
the caller has process context, but scheduling a workqueue to execute
in process context if the caller doesn't have it.

Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

Index: BUILD-2.6/include/linux/workqueue.h
===================================================================
--- BUILD-2.6.orig/include/linux/workqueue.h	2006-02-20 08:57:43.000000000 -0600
+++ BUILD-2.6/include/linux/workqueue.h	2006-02-20 08:58:34.000000000 -0600
@@ -20,6 +20,10 @@
 	struct timer_list timer;
 };
 
+struct execute_work {
+	struct work_struct work;
+};
+
 #define __WORK_INITIALIZER(n, f, d) {				\
         .entry	= { &(n).entry, &(n).entry },			\
 	.func = (f),						\
@@ -74,6 +78,8 @@
 void cancel_rearming_delayed_work(struct work_struct *work);
 void cancel_rearming_delayed_workqueue(struct workqueue_struct *,
 				       struct work_struct *);
+int execute_in_process_context(void (*fn)(void *), void *,
+			       struct execute_work *);
 
 /*
  * Kill off a pending schedule_delayed_work().  Note that the work callback
Index: BUILD-2.6/kernel/workqueue.c
===================================================================
--- BUILD-2.6.orig/kernel/workqueue.c	2006-02-20 08:57:43.000000000 -0600
+++ BUILD-2.6/kernel/workqueue.c	2006-02-20 08:59:18.000000000 -0600
@@ -27,6 +27,7 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/kthread.h>
+#include <linux/hardirq.h>
 
 /*
  * The per-CPU workqueue (if single thread, we always use the first
@@ -476,6 +477,34 @@
 }
 EXPORT_SYMBOL(cancel_rearming_delayed_work);
 
+/**
+ * execute_in_process_context - reliably execute the routine with user context
+ * @fn:		the function to execute
+ * @data:	data to pass to the function
+ * @ew:		guaranteed storage for the execute work structure (must
+ *		be available when the work executes)
+ *
+ * Executes the function immediately if process context is available,
+ * otherwise schedules the function for delayed execution.
+ *
+ * Returns:	0 - function was executed
+ *		1 - function was scheduled for execution
+ */
+int execute_in_process_context(void (*fn)(void *data), void *data,
+			       struct execute_work *ew)
+{
+	if (!in_interrupt()) {
+		fn(data);
+		return 0;
+	}
+
+	INIT_WORK(&ew->work, fn, data);
+	schedule_work(&ew->work);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(execute_in_process_context);
+
 int keventd_up(void)
 {
 	return keventd_wq != NULL;

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[Alan Stern]

On Fri, 17 Feb 2006, James Bottomley wrote:

> +/**
> + * execute_in_process_context - reliably execute the routine with user context
> + * @fn:		the function to execute
> + * @data:	data to pass to the function
> + *
> + * Executes the function immediately if process context is available,
> + * otherwise schedules the function for delayed execution.
> + *
> + * Returns:	0 - function was executed
> + *		1 - function was scheduled for execution
> + */
> +int execute_in_process_context(void (*fn)(void *data), void *data,
> +			       struct execute_work *ew)
> +{
> +	if (!in_interrupt()) {
> +		fn(data);
> +		return 0;
> +	}

The test should be in_atomic(), not in_interrupt().

Alan Stern

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[James Bottomley]

On Sat, 2006-02-18 at 15:16 -0500, Alan Stern wrote:
> The test should be in_atomic(), not in_interrupt().

There's a long prior discussion of why it has to be in_interrupt()

James

Re: Linux 2.6.16-rc3

[Helge Hafting]

Andrew Morton wrote:

>We still have some serious bugs, several of which are in 2.6.15 as well:
>
[...]

>- Helge Hafting reports a usb printer regression - I don't know if that's
>  still live?
>  
>
I tried printing four pages of graphichs with 2.6.16-rc3-mm1
and it worked fine.  When I had the problem I couldn't even print
three, I had to print the 10 pages I needed one by one.

So I believe the situation has improved.

Helge Hafting

RE: [linux-usb-devel] Re: Linux 2.6.16-rc3

[Yu, Luming]

>> I don't think anybody claimed this isn't a regression for the 600X.
>
>I narrowed it further.  The short story is that this commit (diff below
>sig) makes the second S3 sleep go into the endless loop, if the loaded
>modules are exactly thermal, processor, intel_agp, and agpgart:

If you believe this patch is the root cause of the regression you have
been seeing. Then, I would say the thing is a little bit different with
ec_intr=0 and ec_intr=1. Basically, ec_intr=0 will disable EC related
ACPI interrupt before finishing _Qxx method execution , but ec_intr=1
always enable EC interrupt.  This could cause some hardware/BIOS
events get lost under ec_intr=0, which shouldn't lost.

We need to figure out what's going on here rather than falling back to
ec_intr=0 behavior.

--Luming

Re: [linux-usb-devel] Re: Linux 2.6.16-rc3

[Sanjoy Mahajan]

>>> I don't think anybody claimed this isn't a regression for the 600X.

>> I narrowed it further.  The short story is that this commit (diff
>> below sig) makes the second S3 sleep go into the endless loop, if
>> the loaded modules are exactly thermal, processor, intel_agp, and
>> agpgart:

> If you believe this patch is the root cause of the regression you
> have been seeing.

Not sure if you were waiting for me to say something, but I do believe
the change of default from ec_intr=0 to ec_intr=1 causes the problem
for me.

-Sanjoy

`Never underestimate the evil of which men of power are capable.'
         --Bertrand Russell, _War Crimes in Vietnam_, chapter 1.

RE: [linux-usb-devel] Re: Linux 2.6.16-rc3

[Yu, Luming]

>>>> I don't think anybody claimed this isn't a regression for the 600X.
>
>>> I narrowed it further.  The short story is that this commit (diff
>>> below sig) makes the second S3 sleep go into the endless loop, if
>>> the loaded modules are exactly thermal, processor, intel_agp, and
>>> agpgart:
>
>> If you believe this patch is the root cause of the regression you
>> have been seeing.
>
>Not sure if you were waiting for me to say something, but I do believe
>the change of default from ec_intr=0 to ec_intr=1 causes the problem
>for me.
>

I'm fine if you call this regression. :-) 
The real problem is there could be some bugs hidden by ec_intr=0.
I'm pretty sure your 600X box has thermal related issue that is breaking
2nd S3 suspend. But I don't know how.

ec_intr=1 just get these bug  just exposed on your box, 
and we need to work together to fix them. 

Thanks,
Luming