All of lore.kernel.org
 help / color / mirror / Atom feed
* ahci rmmodable and shouldn't
@ 2010-09-16 21:56 Pedro Francisco
  2010-09-16 21:58 ` Randy Dunlap
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Francisco @ 2010-09-16 21:56 UTC (permalink / raw)
  To: linux-kernel

I reported this here for 2.6.35.4 -- http://lkml.org/lkml/2010/9/7/321 -- but 
still happens on latest -git.

`rmmod ahci' works even if the module is being used (`lsmod' thinks it's not 
being used but rmmoding ahci triggers filesystem erros and I've to force a 
reboot).

I've been told it's the same as plugging a cable. IMO that's what 
/sys/stuff/eject_or_similar is to, not `rmmod ahci'.

I *assume* it was caused by the split between libahci & ahci, since in 2.6.32 
`rmmod ahci' is impossible because it's marked as being in use. Didn't try 
`rmmod -f ahci' but that's not the point.

I'm posting again about this because it doesn't make sense to me the actual 
behaviour.

TIA,
-- 
Pedro

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

* Re: ahci rmmodable and shouldn't
  2010-09-16 21:56 ahci rmmodable and shouldn't Pedro Francisco
@ 2010-09-16 21:58 ` Randy Dunlap
  2010-09-17  9:53   ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Randy Dunlap @ 2010-09-16 21:58 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: linux-kernel, linux-ide


On Thu, September 16, 2010 2:56 pm, Pedro Francisco wrote:
> I reported this here for 2.6.35.4 -- http://lkml.org/lkml/2010/9/7/321 --
> but still happens on latest -git.

Please copy linux-ide@vger.kernel.org on ATA issues.
(done now)

> `rmmod ahci' works even if the module is being used (`lsmod' thinks it's
> not being used but rmmoding ahci triggers filesystem erros and I've to
> force a reboot).
>
> I've been told it's the same as plugging a cable. IMO that's what
> /sys/stuff/eject_or_similar is to, not `rmmod ahci'.
>
>
> I *assume* it was caused by the split between libahci & ahci, since in
> 2.6.32
> `rmmod ahci' is impossible because it's marked as being in use. Didn't try
>  `rmmod -f ahci' but that's not the point.
>
>
> I'm posting again about this because it doesn't make sense to me the
> actual behaviour.
>
> TIA,
> --
> Pedro
> --

-- 
~Randy


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

* Re: ahci rmmodable and shouldn't
  2010-09-16 21:58 ` Randy Dunlap
@ 2010-09-17  9:53   ` Tejun Heo
  2010-09-19 10:28     ` Pedro Francisco
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-17  9:53 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Pedro Francisco, linux-kernel, linux-ide

Hello,

On 09/16/2010 11:58 PM, Randy Dunlap wrote:
>> `rmmod ahci' works even if the module is being used (`lsmod' thinks it's
>> not being used but rmmoding ahci triggers filesystem erros and I've to
>> force a reboot).
>>
>> I've been told it's the same as plugging a cable. IMO that's what
>> /sys/stuff/eject_or_similar is to, not `rmmod ahci'.
>>
>>
>> I *assume* it was caused by the split between libahci & ahci, since in
>> 2.6.32
>> `rmmod ahci' is impossible because it's marked as being in use. Didn't try
>>  `rmmod -f ahci' but that's not the point.
>>
>>
>> I'm posting again about this because it doesn't make sense to me the
>> actual behaviour.

Just don't rmmod a module which is serving a live filesystem.  Why
does it even matter?

-- 
tejun

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

* Re: ahci rmmodable and shouldn't
  2010-09-17  9:53   ` Tejun Heo
@ 2010-09-19 10:28     ` Pedro Francisco
  2010-09-19 12:32       ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Francisco @ 2010-09-19 10:28 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Randy Dunlap, linux-kernel, linux-ide

On Sex, 2010-09-17 at 11:53 +0200, Tejun Heo wrote:
> Just don't rmmod a module which is serving a live filesystem.  Why
> does it even matter?
> 

... Ok, let me argue another way. What if my mail was called: «lsmod
shows false values ("Used by: 0") for module 'ahci'», would you agree it
was an error and should therefore be fixed?

-- 
Pedro


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

* Re: ahci rmmodable and shouldn't
  2010-09-19 10:28     ` Pedro Francisco
@ 2010-09-19 12:32       ` Tejun Heo
  2010-09-20  9:25         ` Pedro Francisco
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-19 12:32 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: Randy Dunlap, linux-kernel, linux-ide

Hello,

On 09/19/2010 12:28 PM, Pedro Francisco wrote:
> On Sex, 2010-09-17 at 11:53 +0200, Tejun Heo wrote:
>> Just don't rmmod a module which is serving a live filesystem.  Why
>> does it even matter?
> 
> ... Ok, let me argue another way. What if my mail was called: «lsmod
> shows false values ("Used by: 0") for module 'ahci'», would you agree it
> was an error and should therefore be fixed?

I think it works the same way for any other SCSI driver and it's
pretty convenient during development.  You can unload any network
driver module while the network is still up too.  Module reference
count doesn't have any specific meaning of 'use' preassigned other
than it's there to prevent unloading module while the code and data
are being actively used by the processor.  It's unfortunate that the
way module reference count is used for libata is causing discomfort to
you but I need much better reason to change it.

Thanks.

-- 
tejun

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

* Re: ahci rmmodable and shouldn't
  2010-09-19 12:32       ` Tejun Heo
@ 2010-09-20  9:25         ` Pedro Francisco
  2010-09-20  9:31           ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Francisco @ 2010-09-20  9:25 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, linux-ide

On Dom, 2010-09-19 at 14:32 +0200, Tejun Heo wrote: 
> I think it works the same way for any other SCSI driver and it's
> pretty convenient during development.  You can unload any network
> driver module while the network is still up too.  Module reference
> count doesn't have any specific meaning of 'use' preassigned other
> than it's there to prevent unloading module while the code and data
> are being actively used by the processor.  It's unfortunate that the
> way module reference count is used for libata is causing discomfort to
> you but I need much better reason to change it.

Discomfort? Not really, sorry if my stubbornness felt like it. No, I'm
fine with it.

I just thought it was weird the change of behaviour from 2.6.32, plus
felt that the "Used by" field by lsmod was incorrectly set at 0 since
the module is, in fact, being used (e)

If you say both behaviours are expected and preferred, besides being
consistent with other modules (something I didn't know), I really can't
argue any more.

Thanks for the explanation and sorry for the noise :)

-- 
Pedro


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

* Re: ahci rmmodable and shouldn't
  2010-09-20  9:25         ` Pedro Francisco
@ 2010-09-20  9:31           ` Tejun Heo
  2010-09-20  9:41             ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-20  9:31 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: linux-kernel, linux-ide

Hello,

On 09/20/2010 11:25 AM, Pedro Francisco wrote:
> Discomfort? Not really, sorry if my stubbornness felt like it. No, I'm
> fine with it.

:-)

> I just thought it was weird the change of behaviour from 2.6.32, plus
> felt that the "Used by" field by lsmod was incorrectly set at 0 since
> the module is, in fact, being used (e)

Hmmm... I don't recall the behavior changing from 2.6.32.  Maybe I've
been just speaking bullshit this whole time.  Can you please post the
output of lsmod?  Maybe the difference is caused by the split between
libahci and ahci?

Thanks.

-- 
tejun

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

* Re: ahci rmmodable and shouldn't
  2010-09-20  9:31           ` Tejun Heo
@ 2010-09-20  9:41             ` Tejun Heo
  2010-09-20 20:44               ` Pedro Francisco
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-20  9:41 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: linux-kernel, linux-ide

Hello, again.

Yeah, now I'm fairly sure I was spewing out complete bullshit.  Sorry
about that.  Gees, why does spewing out bullshit always feel so right?
Please feel free to call me an idiot.  Can you please try the
following patch and see whether it changes the module counting
behavior?

Thanks.

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ff1c945..99d0e5a 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -90,6 +90,10 @@ static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif

+static struct scsi_host_template ahci_sht = {
+	AHCI_SHT("ahci"),
+};
+
 static struct ata_port_operations ahci_vt8251_ops = {
 	.inherits		= &ahci_ops,
 	.hardreset		= ahci_vt8251_hardreset,
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 474427b..e5fdeeb 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -298,7 +298,17 @@ struct ahci_host_priv {

 extern int ahci_ignore_sss;

-extern struct scsi_host_template ahci_sht;
+extern struct device_attribute *ahci_shost_attrs[];
+extern struct device_attribute *ahci_sdev_attrs[];
+
+#define AHCI_SHT(drv_name)						\
+	ATA_NCQ_SHT(drv_name),						\
+	.can_queue		= AHCI_MAX_CMDS - 1,			\
+	.sg_tablesize		= AHCI_MAX_SG,				\
+	.dma_boundary		= AHCI_DMA_BOUNDARY,			\
+	.shost_attrs		= ahci_shost_attrs,			\
+	.sdev_attrs		= ahci_sdev_attrs
+
 extern struct ata_port_operations ahci_ops;

 void ahci_save_initial_config(struct device *dev,
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 4e97f33..84b6432 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,10 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"

+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT("ahci_platform"),
+};
+
 static int __init ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -145,7 +149,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 	ahci_print_info(host, "platform");

 	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
-			       &ahci_sht);
+			       &ahci_platform_sht);
 	if (rc)
 		goto err0;

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 68dc678..8eea309 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -121,7 +121,7 @@ static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
 static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
 		   ahci_read_em_buffer, ahci_store_em_buffer);

-static struct device_attribute *ahci_shost_attrs[] = {
+struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_link_power_management_policy,
 	&dev_attr_em_message_type,
 	&dev_attr_em_message,
@@ -132,22 +132,14 @@ static struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_em_buffer,
 	NULL
 };
+EXPORT_SYMBOL_GPL(ahci_shost_attrs);

-static struct device_attribute *ahci_sdev_attrs[] = {
+struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
 	&dev_attr_unload_heads,
 	NULL
 };
-
-struct scsi_host_template ahci_sht = {
-	ATA_NCQ_SHT("ahci"),
-	.can_queue		= AHCI_MAX_CMDS - 1,
-	.sg_tablesize		= AHCI_MAX_SG,
-	.dma_boundary		= AHCI_DMA_BOUNDARY,
-	.shost_attrs		= ahci_shost_attrs,
-	.sdev_attrs		= ahci_sdev_attrs,
-};
-EXPORT_SYMBOL_GPL(ahci_sht);
+EXPORT_SYMBOL_GPL(ahci_sdev_attrs);

 struct ata_port_operations ahci_ops = {
 	.inherits		= &sata_pmp_port_ops,

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

* Re: ahci rmmodable and shouldn't
  2010-09-20  9:41             ` Tejun Heo
@ 2010-09-20 20:44               ` Pedro Francisco
  2010-09-20 22:09                 ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Francisco @ 2010-09-20 20:44 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, linux-ide

On Seg, 2010-09-20 at 11:41 +0200, Tejun Heo wrote:
> Hello, again.
> 
> Yeah, now I'm fairly sure I was spewing out complete bullshit.  Sorry
> about that.  Gees, why does spewing out bullshit always feel so right?
> Please feel free to call me an idiot.  Can you please try the
> following patch and see whether it changes the module counting
> behavior?
> 
> Thanks.
> 
-snip

To which tree shall I apply it? It currently fails both on Linus'
linux-2.6 and
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git .


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

* Re: ahci rmmodable and shouldn't
  2010-09-20 20:44               ` Pedro Francisco
@ 2010-09-20 22:09                 ` Tejun Heo
  2010-09-20 22:10                   ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-20 22:09 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: linux-kernel, linux-ide

Hello,

On 09/20/2010 10:44 PM, Pedro Francisco wrote:
> To which tree shall I apply it? It currently fails both on Linus'
> linux-2.6 and
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git .

Hmm... weird.  It's against Linus' tree.  Checking, yeap, it applies
cleanly to 2.6.36-rc4, commit 2422084a (Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/mattst88/alpha-2.6).

Thanks.

-- 
tejun

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

* Re: ahci rmmodable and shouldn't
  2010-09-20 22:09                 ` Tejun Heo
@ 2010-09-20 22:10                   ` Tejun Heo
  2010-09-21  6:14                     ` Michael Tokarev
  2010-09-23 15:14                     ` ahci rmmodable and shouldn't Pedro Francisco
  0 siblings, 2 replies; 16+ messages in thread
From: Tejun Heo @ 2010-09-20 22:10 UTC (permalink / raw)
  To: Pedro Francisco; +Cc: linux-kernel, linux-ide

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

On 09/21/2010 12:09 AM, Tejun Heo wrote:
> Hello,
> 
> On 09/20/2010 10:44 PM, Pedro Francisco wrote:
>> To which tree shall I apply it? It currently fails both on Linus'
>> linux-2.6 and
>> git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git .
> 
> Hmm... weird.  It's against Linus' tree.  Checking, yeap, it applies
> cleanly to 2.6.36-rc4, commit 2422084a (Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/mattst88/alpha-2.6).

Attaching the patch just in case.

-- 
tejun

[-- Attachment #2: ahci-sht-fix.patch --]
[-- Type: text/x-patch, Size: 3148 bytes --]

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ff1c945..99d0e5a 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -90,6 +90,10 @@ static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif
 
+static struct scsi_host_template ahci_sht = {
+	AHCI_SHT("ahci"),
+};
+
 static struct ata_port_operations ahci_vt8251_ops = {
 	.inherits		= &ahci_ops,
 	.hardreset		= ahci_vt8251_hardreset,
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 474427b..e5fdeeb 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -298,7 +298,17 @@ struct ahci_host_priv {
 
 extern int ahci_ignore_sss;
 
-extern struct scsi_host_template ahci_sht;
+extern struct device_attribute *ahci_shost_attrs[];
+extern struct device_attribute *ahci_sdev_attrs[];
+
+#define AHCI_SHT(drv_name)						\
+	ATA_NCQ_SHT(drv_name),						\
+	.can_queue		= AHCI_MAX_CMDS - 1,			\
+	.sg_tablesize		= AHCI_MAX_SG,				\
+	.dma_boundary		= AHCI_DMA_BOUNDARY,			\
+	.shost_attrs		= ahci_shost_attrs,			\
+	.sdev_attrs		= ahci_sdev_attrs
+
 extern struct ata_port_operations ahci_ops;
 
 void ahci_save_initial_config(struct device *dev,
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 4e97f33..84b6432 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,10 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"
 
+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT("ahci_platform"),
+};
+
 static int __init ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -145,7 +149,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 	ahci_print_info(host, "platform");
 
 	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
-			       &ahci_sht);
+			       &ahci_platform_sht);
 	if (rc)
 		goto err0;
 
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 68dc678..8eea309 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -121,7 +121,7 @@ static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
 static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
 		   ahci_read_em_buffer, ahci_store_em_buffer);
 
-static struct device_attribute *ahci_shost_attrs[] = {
+struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_link_power_management_policy,
 	&dev_attr_em_message_type,
 	&dev_attr_em_message,
@@ -132,22 +132,14 @@ static struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_em_buffer,
 	NULL
 };
+EXPORT_SYMBOL_GPL(ahci_shost_attrs);
 
-static struct device_attribute *ahci_sdev_attrs[] = {
+struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
 	&dev_attr_unload_heads,
 	NULL
 };
-
-struct scsi_host_template ahci_sht = {
-	ATA_NCQ_SHT("ahci"),
-	.can_queue		= AHCI_MAX_CMDS - 1,
-	.sg_tablesize		= AHCI_MAX_SG,
-	.dma_boundary		= AHCI_DMA_BOUNDARY,
-	.shost_attrs		= ahci_shost_attrs,
-	.sdev_attrs		= ahci_sdev_attrs,
-};
-EXPORT_SYMBOL_GPL(ahci_sht);
+EXPORT_SYMBOL_GPL(ahci_sdev_attrs);
 
 struct ata_port_operations ahci_ops = {
 	.inherits		= &sata_pmp_port_ops,

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

* Re: ahci rmmodable and shouldn't
  2010-09-20 22:10                   ` Tejun Heo
@ 2010-09-21  6:14                     ` Michael Tokarev
  2010-09-21  7:25                       ` [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split Tejun Heo
  2010-09-23 15:14                     ` ahci rmmodable and shouldn't Pedro Francisco
  1 sibling, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2010-09-21  6:14 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Pedro Francisco, linux-kernel, linux-ide

21.09.2010 02:10, Tejun Heo wrote:
> On 09/21/2010 12:09 AM, Tejun Heo wrote:
>> Hello,
>>
>> On 09/20/2010 10:44 PM, Pedro Francisco wrote:
>>> To which tree shall I apply it? It currently fails both on Linus'
>>> linux-2.6 and
>>> git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git .
>>
>> Hmm... weird.  It's against Linus' tree.  Checking, yeap, it applies
>> cleanly to 2.6.36-rc4, commit 2422084a (Merge branch 'for-linus' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/mattst88/alpha-2.6).
> 
> Attaching the patch just in case.

I tried it on 2.6.35 kernel.  The ahci module usage count now
increases and decreases properly, at least from what I see here.
That's definitely better than previous 2.6.35 variant.

Thanks!

/mjt


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

* [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split
  2010-09-21  6:14                     ` Michael Tokarev
@ 2010-09-21  7:25                       ` Tejun Heo
  2010-09-21  7:32                         ` Michael Tokarev
  0 siblings, 1 reply; 16+ messages in thread
From: Tejun Heo @ 2010-09-21  7:25 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Michael Tokarev, Pedro Francisco, linux-kernel, linux-ide

libata depends on scsi_host_template for module reference counting and
sht's should be owned by each low level driver.  During libahci split,
the sht was left with libahci.ko leaving the actual low level drivers
not reference counted.  This made ahci and ahci_platform always
unloadable even while they're being actively used.

Fix it by defining AHCI_SHT() macro in ahci.h and defining a sht for
each low level ahci driver.

stable: only applicable to 2.6.35.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Cc: stable@kernel.org
---
Michael, thanks a lot for spotting it and bearing with me.

 drivers/ata/ahci.c          |    4 ++++
 drivers/ata/ahci.h          |   12 +++++++++++-
 drivers/ata/ahci_platform.c |    6 +++++-
 drivers/ata/libahci.c       |   16 ++++------------
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ff1c945..99d0e5a 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -90,6 +90,10 @@ static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int ahci_pci_device_resume(struct pci_dev *pdev);
 #endif

+static struct scsi_host_template ahci_sht = {
+	AHCI_SHT("ahci"),
+};
+
 static struct ata_port_operations ahci_vt8251_ops = {
 	.inherits		= &ahci_ops,
 	.hardreset		= ahci_vt8251_hardreset,
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 474427b..e5fdeeb 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -298,7 +298,17 @@ struct ahci_host_priv {

 extern int ahci_ignore_sss;

-extern struct scsi_host_template ahci_sht;
+extern struct device_attribute *ahci_shost_attrs[];
+extern struct device_attribute *ahci_sdev_attrs[];
+
+#define AHCI_SHT(drv_name)						\
+	ATA_NCQ_SHT(drv_name),						\
+	.can_queue		= AHCI_MAX_CMDS - 1,			\
+	.sg_tablesize		= AHCI_MAX_SG,				\
+	.dma_boundary		= AHCI_DMA_BOUNDARY,			\
+	.shost_attrs		= ahci_shost_attrs,			\
+	.sdev_attrs		= ahci_sdev_attrs
+
 extern struct ata_port_operations ahci_ops;

 void ahci_save_initial_config(struct device *dev,
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 4e97f33..84b6432 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -23,6 +23,10 @@
 #include <linux/ahci_platform.h>
 #include "ahci.h"

+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT("ahci_platform"),
+};
+
 static int __init ahci_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -145,7 +149,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 	ahci_print_info(host, "platform");

 	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
-			       &ahci_sht);
+			       &ahci_platform_sht);
 	if (rc)
 		goto err0;

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 68dc678..8eea309 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -121,7 +121,7 @@ static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
 static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
 		   ahci_read_em_buffer, ahci_store_em_buffer);

-static struct device_attribute *ahci_shost_attrs[] = {
+struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_link_power_management_policy,
 	&dev_attr_em_message_type,
 	&dev_attr_em_message,
@@ -132,22 +132,14 @@ static struct device_attribute *ahci_shost_attrs[] = {
 	&dev_attr_em_buffer,
 	NULL
 };
+EXPORT_SYMBOL_GPL(ahci_shost_attrs);

-static struct device_attribute *ahci_sdev_attrs[] = {
+struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
 	&dev_attr_unload_heads,
 	NULL
 };
-
-struct scsi_host_template ahci_sht = {
-	ATA_NCQ_SHT("ahci"),
-	.can_queue		= AHCI_MAX_CMDS - 1,
-	.sg_tablesize		= AHCI_MAX_SG,
-	.dma_boundary		= AHCI_DMA_BOUNDARY,
-	.shost_attrs		= ahci_shost_attrs,
-	.sdev_attrs		= ahci_sdev_attrs,
-};
-EXPORT_SYMBOL_GPL(ahci_sht);
+EXPORT_SYMBOL_GPL(ahci_sdev_attrs);

 struct ata_port_operations ahci_ops = {
 	.inherits		= &sata_pmp_port_ops,

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

* Re: [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split
  2010-09-21  7:25                       ` [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split Tejun Heo
@ 2010-09-21  7:32                         ` Michael Tokarev
  2010-09-21  8:17                           ` Tejun Heo
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2010-09-21  7:32 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, Pedro Francisco, linux-kernel, linux-ide

21.09.2010 11:25, Tejun Heo wrote:
> libata depends on scsi_host_template for module reference counting and
> sht's should be owned by each low level driver.  During libahci split,
> the sht was left with libahci.ko leaving the actual low level drivers
> not reference counted.  This made ahci and ahci_platform always
> unloadable even while they're being actively used.
> 
> Fix it by defining AHCI_SHT() macro in ahci.h and defining a sht for
> each low level ahci driver.
> 
> stable: only applicable to 2.6.35.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> Cc: stable@kernel.org
> ---
> Michael, thanks a lot for spotting it and bearing with me.

Um.  All thanks goes to Pedro Francisco please, not to me -
I just verified your patch on my system, it was Pedro who spotted
and reported this.  You may add my

 Tested-Off-By: Michael Tokarev <mjt@tls.msk.ru>

, but not Reported-by... please ;)

Thanks!

/mjt

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

* Re: [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split
  2010-09-21  7:32                         ` Michael Tokarev
@ 2010-09-21  8:17                           ` Tejun Heo
  0 siblings, 0 replies; 16+ messages in thread
From: Tejun Heo @ 2010-09-21  8:17 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Jeff Garzik, Pedro Francisco, linux-kernel, linux-ide

On 09/21/2010 09:32 AM, Michael Tokarev wrote:
> 21.09.2010 11:25, Tejun Heo wrote:
>> libata depends on scsi_host_template for module reference counting and
>> sht's should be owned by each low level driver.  During libahci split,
>> the sht was left with libahci.ko leaving the actual low level drivers
>> not reference counted.  This made ahci and ahci_platform always
>> unloadable even while they're being actively used.
>>
>> Fix it by defining AHCI_SHT() macro in ahci.h and defining a sht for
>> each low level ahci driver.
>>
>> stable: only applicable to 2.6.35.
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>> Cc: stable@kernel.org
>> ---
>> Michael, thanks a lot for spotting it and bearing with me.
> Um.  All thanks goes to Pedro Francisco please, not to me -
> I just verified your patch on my system, it was Pedro who spotted
> and reported this.  You may add my
> 
>  Tested-Off-By: Michael Tokarev <mjt@tls.msk.ru>
> 
> , but not Reported-by... please ;)

Oh, right.  Jeff, can you please adjust tags to

  Reported-by: Pedro Francisco <pedrogfrancisco@gmail.com>
  Tested-by: Michael Tokarev <mjt@tls.msk.ru>

Thanks.

-- 
tejun

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

* Re: ahci rmmodable and shouldn't
  2010-09-20 22:10                   ` Tejun Heo
  2010-09-21  6:14                     ` Michael Tokarev
@ 2010-09-23 15:14                     ` Pedro Francisco
  1 sibling, 0 replies; 16+ messages in thread
From: Pedro Francisco @ 2010-09-23 15:14 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-kernel, linux-ide

On Ter, 2010-09-21 at 00:10 +0200, Tejun Heo wrote:
> On 09/21/2010 12:09 AM, Tejun Heo wrote:
> > Hello,
> > 
> > On 09/20/2010 10:44 PM, Pedro Francisco wrote:
> >> To which tree shall I apply it? It currently fails both on Linus'
> >> linux-2.6 and
> >> git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git .
> > 
> > Hmm... weird.  It's against Linus' tree.  Checking, yeap, it applies
> > cleanly to 2.6.36-rc4, commit 2422084a (Merge branch 'for-linus' of
> > git://git.kernel.org/pub/scm/linux/kernel/git/mattst88/alpha-2.6).
> 
> Attaching the patch just in case.

I must have forgotten "-p1" since it now worked fine.

Anyway, the patch works on 2.6.36-rc4 :)

Sorry for the late reply but compiling and running it took a bit longer
than expected.
-- 
Pedro

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

end of thread, other threads:[~2010-09-23 15:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-16 21:56 ahci rmmodable and shouldn't Pedro Francisco
2010-09-16 21:58 ` Randy Dunlap
2010-09-17  9:53   ` Tejun Heo
2010-09-19 10:28     ` Pedro Francisco
2010-09-19 12:32       ` Tejun Heo
2010-09-20  9:25         ` Pedro Francisco
2010-09-20  9:31           ` Tejun Heo
2010-09-20  9:41             ` Tejun Heo
2010-09-20 20:44               ` Pedro Francisco
2010-09-20 22:09                 ` Tejun Heo
2010-09-20 22:10                   ` Tejun Heo
2010-09-21  6:14                     ` Michael Tokarev
2010-09-21  7:25                       ` [PATCH #upstream-fixes] ahci: fix module refcount breakage introduced by libahci split Tejun Heo
2010-09-21  7:32                         ` Michael Tokarev
2010-09-21  8:17                           ` Tejun Heo
2010-09-23 15:14                     ` ahci rmmodable and shouldn't Pedro Francisco

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.