* [PATCH] eHEA: Don't do memory allocation under lock if not necessary
@ 2009-03-11 8:44 David Howells
2009-03-11 9:37 ` Jan-Bernd Themann
0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2009-03-11 8:44 UTC (permalink / raw)
To: raisch, themann, tklein; +Cc: linuxppc-dev, netdev
In ehea_probe_adapter() the initial memory allocation and initialisation does
not need to be done with the ehea_fw_handles.lock semaphore held. Doing so
extends the amount of time the lock is held unnecessarily.
Signed-off-by: David Howells <dhowells@redhat.com>
---
drivers/net/ehea/ehea_main.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index dfe9226..34480ae 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -3370,18 +3370,19 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
ehea_error("Invalid ibmebus device probed");
return -EINVAL;
}
- mutex_lock(&ehea_fw_handles.lock);
adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
if (!adapter) {
- ret = -ENOMEM;
dev_err(&dev->dev, "no mem for ehea_adapter\n");
- goto out;
+ return -ENOMEM;
}
- list_add(&adapter->list, &adapter_list);
-
adapter->ofdev = dev;
+ adapter->pd = EHEA_PD_ID;
+
+ mutex_lock(&ehea_fw_handles.lock);
+
+ list_add(&adapter->list, &adapter_list);
adapter_handle = of_get_property(dev->node, "ibm,hea-handle",
NULL);
@@ -3395,8 +3396,6 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
goto out_free_ad;
}
- adapter->pd = EHEA_PD_ID;
-
dev->dev.driver_data = adapter;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] eHEA: Don't do memory allocation under lock if not necessary
2009-03-11 8:44 [PATCH] eHEA: Don't do memory allocation under lock if not necessary David Howells
@ 2009-03-11 9:37 ` Jan-Bernd Themann
0 siblings, 0 replies; 4+ messages in thread
From: Jan-Bernd Themann @ 2009-03-11 9:37 UTC (permalink / raw)
To: David Howells; +Cc: tklein, linuxppc-dev, raisch, netdev, themann
Hi David,
thanks for your patch. Coincidentally we have been working on a patch that
does some locking rework and also touches this particular lock.
So your patch finnally won't be required anymore. Thanks anyway for trying
to improve the eHEA driver!
I'm going to post our patch later today.
Regards,
Jan-Bernd
On Wednesday 11 March 2009 09:44:57 David Howells wrote:
> In ehea_probe_adapter() the initial memory allocation and initialisation does
> not need to be done with the ehea_fw_handles.lock semaphore held. Doing so
> extends the amount of time the lock is held unnecessarily.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> drivers/net/ehea/ehea_main.c | 13 ++++++-------
> 1 files changed, 6 insertions(+), 7 deletions(-)
>
>
> diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
> index dfe9226..34480ae 100644
> --- a/drivers/net/ehea/ehea_main.c
> +++ b/drivers/net/ehea/ehea_main.c
> @@ -3370,18 +3370,19 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
> ehea_error("Invalid ibmebus device probed");
> return -EINVAL;
> }
> - mutex_lock(&ehea_fw_handles.lock);
>
> adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
> if (!adapter) {
> - ret = -ENOMEM;
> dev_err(&dev->dev, "no mem for ehea_adapter\n");
> - goto out;
> + return -ENOMEM;
> }
>
> - list_add(&adapter->list, &adapter_list);
> -
> adapter->ofdev = dev;
> + adapter->pd = EHEA_PD_ID;
> +
> + mutex_lock(&ehea_fw_handles.lock);
> +
> + list_add(&adapter->list, &adapter_list);
>
> adapter_handle = of_get_property(dev->node, "ibm,hea-handle",
> NULL);
> @@ -3395,8 +3396,6 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
> goto out_free_ad;
> }
>
> - adapter->pd = EHEA_PD_ID;
> -
> dev->dev.driver_data = adapter;
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] eHEA: Don't do memory allocation under lock if not necessary
@ 2009-03-09 16:36 David Howells
2009-03-11 4:10 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2009-03-09 16:36 UTC (permalink / raw)
To: raisch, themann, tklein; +Cc: linuxppc-dev
In ehea_probe_adapter() the initial memory allocation and initialisation does
not need to be done with the ehea_fw_handles.lock semaphore held. Doing so
extends the amount of time the lock is held unnecessarily.
Signed-off-by: David Howells <dhowells@redhat.com>
---
drivers/net/ehea/ehea_main.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index dfe9226..34480ae 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -3370,18 +3370,19 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
ehea_error("Invalid ibmebus device probed");
return -EINVAL;
}
- mutex_lock(&ehea_fw_handles.lock);
adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
if (!adapter) {
- ret = -ENOMEM;
dev_err(&dev->dev, "no mem for ehea_adapter\n");
- goto out;
+ return -ENOMEM;
}
- list_add(&adapter->list, &adapter_list);
-
adapter->ofdev = dev;
+ adapter->pd = EHEA_PD_ID;
+
+ mutex_lock(&ehea_fw_handles.lock);
+
+ list_add(&adapter->list, &adapter_list);
adapter_handle = of_get_property(dev->node, "ibm,hea-handle",
NULL);
@@ -3395,8 +3396,6 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
goto out_free_ad;
}
- adapter->pd = EHEA_PD_ID;
-
dev->dev.driver_data = adapter;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] eHEA: Don't do memory allocation under lock if not necessary
2009-03-09 16:36 David Howells
@ 2009-03-11 4:10 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2009-03-11 4:10 UTC (permalink / raw)
To: David Howells; +Cc: tklein, linuxppc-dev, raisch, themann
On Mon, 2009-03-09 at 16:36 +0000, David Howells wrote:
> In ehea_probe_adapter() the initial memory allocation and initialisation does
> not need to be done with the ehea_fw_handles.lock semaphore held. Doing so
> extends the amount of time the lock is held unnecessarily.
Can you resend with netdev on CC ?
Cheers,
Ben.
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> drivers/net/ehea/ehea_main.c | 13 ++++++-------
> 1 files changed, 6 insertions(+), 7 deletions(-)
>
>
> diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
> index dfe9226..34480ae 100644
> --- a/drivers/net/ehea/ehea_main.c
> +++ b/drivers/net/ehea/ehea_main.c
> @@ -3370,18 +3370,19 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
> ehea_error("Invalid ibmebus device probed");
> return -EINVAL;
> }
> - mutex_lock(&ehea_fw_handles.lock);
>
> adapter = kzalloc(sizeof(*adapter), GFP_KERNEL);
> if (!adapter) {
> - ret = -ENOMEM;
> dev_err(&dev->dev, "no mem for ehea_adapter\n");
> - goto out;
> + return -ENOMEM;
> }
>
> - list_add(&adapter->list, &adapter_list);
> -
> adapter->ofdev = dev;
> + adapter->pd = EHEA_PD_ID;
> +
> + mutex_lock(&ehea_fw_handles.lock);
> +
> + list_add(&adapter->list, &adapter_list);
>
> adapter_handle = of_get_property(dev->node, "ibm,hea-handle",
> NULL);
> @@ -3395,8 +3396,6 @@ static int __devinit ehea_probe_adapter(struct of_device *dev,
> goto out_free_ad;
> }
>
> - adapter->pd = EHEA_PD_ID;
> -
> dev->dev.driver_data = adapter;
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-03-11 9:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-11 8:44 [PATCH] eHEA: Don't do memory allocation under lock if not necessary David Howells
2009-03-11 9:37 ` Jan-Bernd Themann
-- strict thread matches above, loose matches on Subject: below --
2009-03-09 16:36 David Howells
2009-03-11 4:10 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).