Re: [PATCH 01/12] libata: allocate ap separately from shost

Re: [PATCH 01/12] libata: allocate ap separately from shost

[Jeff Garzik]

Tejun Heo wrote:
> Don't embed ap inside shost.  Allocate it separately and point it back
> from shosts's hostdata.  This makes port allocation more flexible and
> allows regular ATA and SAS share host alloc/init paths.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |   19 ++++++++++++++-----
>  include/linux/libata.h    |    2 +-
>  2 files changed, 15 insertions(+), 6 deletions(-)

applied patches 1-12 to #upstream.

Yay, libata init model is finally clean!

Thanks,

	Jeff

[PATCHSET] libata: implement new initialization model, take #4

[Tejun Heo]

Hello, all.

This is the fourth take of new-init-model patchset.  Changes from the
last take[L] are...

* memory leak fixed in patch#1
* s/attach/register/g
* ap->cbl == ATA_CBL_* test replaced with ATA_FLAG_SATA test
* updated to the current upstream
* dependency to pci_configure_dma_masks() dropped

This patchset is against

 upstream(c602a03a610d0626599dbfb28e073c58699e5f94)
 + [1] libata-fix-native-mode-disabled-port-handling
 + [2] ahci-implement-ata_save-restore_initial_config
 + [3] ahci-move-port_map-handling-to-ahci_save_initial_config

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.ide/16810
[1] http://article.gmane.org/gmane.linux.ide/16804
[2] http://article.gmane.org/gmane.linux.ide/17302
[3] http://article.gmane.org/gmane.linux.ide/17303

[PATCH 01/12] libata: allocate ap separately from shost

[Tejun Heo]

Don't embed ap inside shost.  Allocate it separately and point it back
from shosts's hostdata.  This makes port allocation more flexible and
allows regular ATA and SAS share host alloc/init paths.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |   19 ++++++++++++++-----
 include/linux/libata.h    |    2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 735f0b0..542e9ea 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5782,13 +5782,18 @@ static struct ata_port * ata_port_add(const struct ata_probe_ent *ent,
 		return NULL;
 	}
 
-	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port));
-	if (!shost)
+	ap = kzalloc(sizeof(struct ata_port), GFP_KERNEL);
+	if (!ap)
 		return NULL;
 
-	shost->transportt = &ata_scsi_transport_template;
+	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port *));
+	if (!shost) {
+		kfree(ap);
+		return NULL;
+	}
 
-	ap = ata_shost_to_port(shost);
+	*(struct ata_port **)&shost->hostdata[0] = ap;
+	shost->transportt = &ata_scsi_transport_template;
 
 	ata_port_init(ap, host, ent, port_no);
 	ata_port_init_shost(ap, shost);
@@ -5814,9 +5819,13 @@ static void ata_host_release(struct device *gendev, void *res)
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
 
-		if (ap)
+		if (!ap)
+			continue;
+
+		if (ap->scsi_host)
 			scsi_host_put(ap->scsi_host);
 
+		kfree(ap);
 		host->ports[i] = NULL;
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 12237d4..ced9dd5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1231,7 +1231,7 @@ static inline void ata_pad_free(struct ata_port *ap, struct device *dev)
 
 static inline struct ata_port *ata_shost_to_port(struct Scsi_Host *host)
 {
-	return (struct ata_port *) &host->hostdata[0];
+	return *(struct ata_port **)&host->hostdata[0];
 }
 
 #endif /* __LINUX_LIBATA_H__ */
-- 
1.5.0.3

[PATCHSET] libata: implement new initialization model, take #3

[Tejun Heo]

Hello, all.

This is the third take of new init model.  Previously, libata LLDs
should have filled probe_ent with appropriate info and pass it to
ata_device_add() which will allocate and initialize ata_host
accordingly, request IRQs and attache the host.

The problem with the previous approach was that the info which can be
passed via probe_ent is inflexible.  This made init path of peculiar
controllers twisted.  For example, sata_promise and sata_via support
heterogeneous configuration where PATA and SATA ports reside on the
same host.  As probe_ent couldn't carry this information to
ata_device_add(), those controllers had to override stuff in
->port_start() and add branches in various places.

New init model is a bit more conventional than the old one.  It
follows the good old allocate-init-attach model.  A LLD allocates a
host populated with the specified number of ports, initializes the
host as it wishes and tell libata to attach the initialized host.  So,
LLDs can intialize the host anyway it wants to.

Helpers to do a series of things in one go are provided such that
simple/common initialization can be performed easily.  Requesting IRQ
is now LLD's responsibility and libata knows nothing about it (except
in activation helper).  This nicely allows MSIX support if ever
needed.

This patchset increases code in libata core layer but makes LLDs
simpler.  For details, please read description in each patch.

As devres is now in place, libata core layer can offload resource
management to it.  This makes this take very different and much
simpler than the last take[L].  As it's a vastly different
implementation, detailed change list isn't very meaningful and
thus omitted.

This patchset is against...

libata-dev#upstream (bbaa9bb11c6854781ac13c64954f42bc0e067c09)
+ [1] libata-fix-ata_host_release-free-order
+ [2] libata-fix-native-mode-disabled-port-handling
+ [3] iomap-implement-pcim-iounmap-regions
+ [4] ahci-separate-port-number-determination-from-host-initialization

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.ide/12741
[1] http://article.gmane.org/gmane.linux.ide/16802
[2] http://article.gmane.org/gmane.linux.ide/16804
[3] http://article.gmane.org/gmane.linux.ide/16805
[4] http://article.gmane.org/gmane.linux.ide/16806

[PATCH 01/12] libata: allocate ap separately from shost

[Tejun Heo]

Don't embed ap inside shost.  Allocate it separately and point it back
from shosts's hostdata.  This makes port allocation more flexible and
allows regular ATA and SAS share host alloc/init paths.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |   15 +++++++++++----
 include/linux/libata.h    |    2 +-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6e124bd..fab488c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5646,14 +5646,17 @@ static struct ata_port * ata_port_add(const struct ata_probe_ent *ent,
 		return NULL;
 	}
 
-	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port));
+	ap = kzalloc(sizeof(struct ata_port), GFP_KERNEL);
+	if (!ap)
+		return NULL;
+
+	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port *));
 	if (!shost)
 		return NULL;
 
+	*(struct ata_port **)&shost->hostdata[0] = ap;
 	shost->transportt = &ata_scsi_transport_template;
 
-	ap = ata_shost_to_port(shost);
-
 	ata_port_init(ap, host, ent, port_no);
 	ata_port_init_shost(ap, shost);
 
@@ -5678,9 +5681,13 @@ static void ata_host_release(struct device *gendev, void *res)
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
 
-		if (ap)
+		if (!ap)
+			continue;
+
+		if (ap->scsi_host)
 			scsi_host_put(ap->scsi_host);
 
+		kfree(ap);
 		host->ports[i] = NULL;
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 911ac4e..cfdc617 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1221,7 +1221,7 @@ static inline void ata_pad_free(struct ata_port *ap, struct device *dev)
 
 static inline struct ata_port *ata_shost_to_port(struct Scsi_Host *host)
 {
-	return (struct ata_port *) &host->hostdata[0];
+	return *(struct ata_port **)&host->hostdata[0];
 }
 
 #endif /* __LINUX_LIBATA_H__ */
-- 
1.5.0.1

Re: [PATCH 01/12] libata: allocate ap separately from shost

[Jeff Garzik]

Tejun Heo wrote:
> Don't embed ap inside shost.  Allocate it separately and point it back
> from shosts's hostdata.  This makes port allocation more flexible and
> allows regular ATA and SAS share host alloc/init paths.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |   15 +++++++++++----
>  include/linux/libata.h    |    2 +-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 6e124bd..fab488c 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5646,14 +5646,17 @@ static struct ata_port * ata_port_add(const struct ata_probe_ent *ent,
>  		return NULL;
>  	}
>  
> -	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port));
> +	ap = kzalloc(sizeof(struct ata_port), GFP_KERNEL);
> +	if (!ap)
> +		return NULL;
> +
> +	shost = scsi_host_alloc(ent->sht, sizeof(struct ata_port *));
>  	if (!shost)
>  		return NULL;

memory leak on error

otherwise OK