[PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Jiri Slaby]

megaraid_sas suspend and resume are inappropriatelly placed in
__devinit section. Remove those placements and make the stuff
dependent on CONFIG_PM.

While at it, mark remove function as __devexit.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Neela Syam Kolli <megaraidlinux@lsi.com>
---
 drivers/scsi/megaraid/megaraid_sas.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index b937e9c..a90944c 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -2650,12 +2650,13 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
 	return;
 }
 
+#ifdef CONFIG_PM
 /**
  * megasas_suspend -	driver suspend entry point
  * @pdev:		PCI device structure
  * @state:		PCI power state to suspend routine
  */
-static int __devinit
+static int
 megasas_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct Scsi_Host *host;
@@ -2687,7 +2688,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
  * megasas_resume-      driver resume entry point
  * @pdev:               PCI device structure
  */
-static int __devinit
+static int
 megasas_resume(struct pci_dev *pdev)
 {
 	int rval;
@@ -2782,12 +2783,16 @@ fail_ready_state:
 
 	return -ENODEV;
 }
+#else
+#define megasas_suspend	NULL
+#define megasas_resume	NULL
+#endif
 
 /**
  * megasas_detach_one -	PCI hot"un"plug entry point
  * @pdev:		PCI device structure
  */
-static void megasas_detach_one(struct pci_dev *pdev)
+static void __devexit megasas_detach_one(struct pci_dev *pdev)
 {
 	int i;
 	struct Scsi_Host *host;
-- 
1.5.4.5

RE: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Yang, Bo]

ACK.

Thanks.

Bo Yang 

-----Original Message-----
From: Jiri Slaby [mailto:jirislaby@gmail.com] 
Sent: Thursday, May 01, 2008 11:56 AM
To: DL-MegaRAID Linux
Cc: linux-scsi@vger.kernel.org; James.Bottomley@HansenPartnership.com;
akpm@linux-foundation.org; linux-kernel@vger.kernel.org; Jiri Slaby
Subject: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

megaraid_sas suspend and resume are inappropriatelly placed in __devinit
section. Remove those placements and make the stuff dependent on
CONFIG_PM.

While at it, mark remove function as __devexit.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Neela Syam Kolli <megaraidlinux@lsi.com>
---
 drivers/scsi/megaraid/megaraid_sas.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.c
b/drivers/scsi/megaraid/megaraid_sas.c
index b937e9c..a90944c 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -2650,12 +2650,13 @@ static void megasas_shutdown_controller(struct
megasas_instance *instance,
 	return;
 }
 
+#ifdef CONFIG_PM
 /**
  * megasas_suspend -	driver suspend entry point
  * @pdev:		PCI device structure
  * @state:		PCI power state to suspend routine
  */
-static int __devinit
+static int
 megasas_suspend(struct pci_dev *pdev, pm_message_t state)  {
 	struct Scsi_Host *host;
@@ -2687,7 +2688,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t
state)
  * megasas_resume-      driver resume entry point
  * @pdev:               PCI device structure
  */
-static int __devinit
+static int
 megasas_resume(struct pci_dev *pdev)
 {
 	int rval;
@@ -2782,12 +2783,16 @@ fail_ready_state:
 
 	return -ENODEV;
 }
+#else
+#define megasas_suspend	NULL
+#define megasas_resume	NULL
+#endif
 
 /**
  * megasas_detach_one -	PCI hot"un"plug entry point
  * @pdev:		PCI device structure
  */
-static void megasas_detach_one(struct pci_dev *pdev)
+static void __devexit megasas_detach_one(struct pci_dev *pdev)
 {
 	int i;
 	struct Scsi_Host *host;
--
1.5.4.5

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Andrew Morton]

On Thu,  1 May 2008 17:56:02 +0200
Jiri Slaby <jirislaby@gmail.com> wrote:

> megaraid_sas suspend and resume are inappropriatelly placed in
> __devinit section.

That's a box-killer, isn't it?

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Jiri Slaby]

On 05/01/2008 11:23 PM, Andrew Morton wrote:
> On Thu,  1 May 2008 17:56:02 +0200
> Jiri Slaby <jirislaby@gmail.com> wrote:
> 
>> megaraid_sas suspend and resume are inappropriatelly placed in
>> __devinit section.
> 
> That's a box-killer, isn't it?

I think so -- the non-CONFIG_HOTPLUG ones.

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[James Bottomley]

On Thu, 2008-05-01 at 23:34 +0200, Jiri Slaby wrote:
> On 05/01/2008 11:23 PM, Andrew Morton wrote:
> > On Thu,  1 May 2008 17:56:02 +0200
> > Jiri Slaby <jirislaby@gmail.com> wrote:
> > 
> >> megaraid_sas suspend and resume are inappropriatelly placed in
> >> __devinit section.
> > 
> > That's a box-killer, isn't it?
> 
> I think so -- the non-CONFIG_HOTPLUG ones.

CONFIG_HOTPLUG is only settable to 'n' if you're CONFIG_EMBEDDED which
has a zero set intersection with the users of megaraid, so in practical
terms, there's no actual box it could kill.

this whole 

#if CONFIG_PM
define suspend resume
#else
set suspend resume methods to null
#endif

Is completely analagous to what we used to do with CONFIG_HOTPLUG before
we had the __dev.* sectional annotations.  Since the expanding
bureacracy is determined to keep the _dev.* sections in spite of the
pain, could we not at least make the machinery do something vaguely
useful and expand it to confine the pm routines to sections which can be
discarded if CONFIG_PM is n?

James

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Andrew Morton]

On Thu, 01 May 2008 17:05:59 -0500
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Thu, 2008-05-01 at 23:34 +0200, Jiri Slaby wrote:
> > On 05/01/2008 11:23 PM, Andrew Morton wrote:
> > > On Thu,  1 May 2008 17:56:02 +0200
> > > Jiri Slaby <jirislaby@gmail.com> wrote:
> > > 
> > >> megaraid_sas suspend and resume are inappropriatelly placed in
> > >> __devinit section.
> > > 
> > > That's a box-killer, isn't it?
> > 
> > I think so -- the non-CONFIG_HOTPLUG ones.
> 
> CONFIG_HOTPLUG is only settable to 'n' if you're CONFIG_EMBEDDED which
> has a zero set intersection with the users of megaraid, so in practical
> terms, there's no actual box it could kill.

who suspends and resumes servers?

> this whole 
> 
> #if CONFIG_PM
> define suspend resume
> #else
> set suspend resume methods to null
> #endif
> 
> Is completely analagous to what we used to do with CONFIG_HOTPLUG before
> we had the __dev.* sectional annotations.  Since the expanding
> bureacracy is determined to keep the _dev.* sections 

ooh, that makes us sound really bad!  ("since the server-obsessed
embedded-hating bloatmonkeys..."?)

> in spite of the pain,

What pain?  Other people write the dang patches for you!  Their main
problem is getting them merged.

> could we not at least make the machinery do something vaguely
> useful and expand it to confine the pm routines to sections which can be
> discarded if CONFIG_PM is n?

a) it would need to be discarded at link-time, ideally.

b) worth investigating.  It might lead to lengthy chains of compilation
   warnings though.

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[James Bottomley]

On Thu, 2008-05-01 at 15:30 -0700, Andrew Morton wrote: 
> On Thu, 01 May 2008 17:05:59 -0500
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > On Thu, 2008-05-01 at 23:34 +0200, Jiri Slaby wrote:
> > > On 05/01/2008 11:23 PM, Andrew Morton wrote:
> > > > On Thu,  1 May 2008 17:56:02 +0200
> > > > Jiri Slaby <jirislaby@gmail.com> wrote:
> > > > 
> > > >> megaraid_sas suspend and resume are inappropriatelly placed in
> > > >> __devinit section.
> > > > 
> > > > That's a box-killer, isn't it?
> > > 
> > > I think so -- the non-CONFIG_HOTPLUG ones.
> > 
> > CONFIG_HOTPLUG is only settable to 'n' if you're CONFIG_EMBEDDED which
> > has a zero set intersection with the users of megaraid, so in practical
> > terms, there's no actual box it could kill.
> 
> who suspends and resumes servers?

Well, IBM for one ... although you wouldn't have to try hard to convince
me that they do it just to annoy me.

> > this whole 
> > 
> > #if CONFIG_PM
> > define suspend resume
> > #else
> > set suspend resume methods to null
> > #endif
> > 
> > Is completely analagous to what we used to do with CONFIG_HOTPLUG before
> > we had the __dev.* sectional annotations.  Since the expanding
> > bureacracy is determined to keep the _dev.* sections 
> 
> ooh, that makes us sound really bad!  ("since the server-obsessed
> embedded-hating bloatmonkeys..."?)

Well, since the __dev.* sections according to the embedded folks are
worth about a page possibly two, the __dev.* value to them is minimal
(particularly as a lot of them have hotplug anyway for their compact
flash, USB and the like).

> > in spite of the pain,
> 
> What pain?  Other people write the dang patches for you!  Their main
> problem is getting them merged.

No, the problem is getting them reviewed.  Since everyone seems to have
section mismatch fatigue they all seem to land on me.

> > could we not at least make the machinery do something vaguely
> > useful and expand it to confine the pm routines to sections which can be
> > discarded if CONFIG_PM is n?
> 
> a) it would need to be discarded at link-time, ideally.

Yes, the same way the exit sections are: as linker discards.  We also
have the mechanics for runtime discards which is useful for modules

> b) worth investigating.  It might lead to lengthy chains of compilation
>    warnings though.

Yes ... but if it was worth the effort to unify all the handrolled
CONFIG_HOTPLUG stuff, it should be worth it for all the handrolled
CONFIG_PM code.

James

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Jiri Slaby]

James Bottomley napsal(a):
>>> could we not at least make the machinery do something vaguely
>>> useful and expand it to confine the pm routines to sections which can be
>>> discarded if CONFIG_PM is n?
>> a) it would need to be discarded at link-time, ideally.
> 
> Yes, the same way the exit sections are: as linker discards.  We also
> have the mechanics for runtime discards which is useful for modules

No it doesn't, exits are discarded at runtime only. At least on x86.

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[James Bottomley]

On Fri, 2008-05-02 at 17:01 +0200, Jiri Slaby wrote:
> James Bottomley napsal(a):
> >>> could we not at least make the machinery do something vaguely
> >>> useful and expand it to confine the pm routines to sections which can be
> >>> discarded if CONFIG_PM is n?
> >> a) it would need to be discarded at link-time, ideally.
> > 
> > Yes, the same way the exit sections are: as linker discards.  We also
> > have the mechanics for runtime discards which is useful for modules
> 
> No it doesn't, exits are discarded at runtime only. At least on x86.

No what doesn't?  I said we have the mechanics to discard both at link
and at run time.

I think you'll find that .exitcall.exit is link time discarded on x86
and the others are runtime discarded.

James

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Zan Lynx]

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

On Thu, 2008-05-01 at 15:30 -0700, Andrew Morton wrote:
[cut] 
> > CONFIG_HOTPLUG is only settable to 'n' if you're CONFIG_EMBEDDED which
> > has a zero set intersection with the users of megaraid, so in practical
> > terms, there's no actual box it could kill.
> 
> who suspends and resumes servers?

I read an article not long ago about a guy running an AMD renderfarm.
He was working on power management techniques, including suspending
systems that weren't in use.  It made a huge difference in power costs,
apparently.

I believe suspend is also used in some virtual server farms where
physical systems are resumed and virtuals are migrated to satisfy higher
CPU demands.

I could also see using it combined with a load balancer or proxy to
bring up backup web or database servers.

Compared to using networked/USB power bars or management controllers to
bring systems up and down, suspend/resume provides a faster response.
-- 
Zan Lynx <zlynx@acm.org>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH #repost] SCSI: megaraid, fix suspend/resume sections

[Pavel Machek]

Hi!

> > > >> megaraid_sas suspend and resume are inappropriatelly placed in
> > > >> __devinit section.
> > > > 
> > > > That's a box-killer, isn't it?
> > > 
> > > I think so -- the non-CONFIG_HOTPLUG ones.
> > 
> > CONFIG_HOTPLUG is only settable to 'n' if you're CONFIG_EMBEDDED which
> > has a zero set intersection with the users of megaraid, so in practical
> > terms, there's no actual box it could kill.
> 
> who suspends and resumes servers?

It actually has some silly name like 'green IT'.
							Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html