* [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu()
@ 2012-08-23 21:35 Tejun Heo
2012-09-14 18:13 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2012-08-23 21:35 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel
pci_call_probe() uses work_on_cpu(), which creates and tears down a
full kthread on each call, to invoke ->probe() on node local CPU for
allocation affinity.
The same goal can easily be achieved using a work item. This patch
rewrites pci_call_probe() so that it uses a work item instead of
work_on_cpu().
Note that the function is restructured for simplicity. This adds
get/put_online_cpus() pair for devices without node but the overhead
of doing so isn't anything material at this level.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
I tested both paths but had to simulate device on foreign node.
Thanks.
drivers/pci/pci-driver.c | 48 +++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 5270f1a..554c15a 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -274,11 +274,14 @@ struct drv_dev_and_id {
struct pci_driver *drv;
struct pci_dev *dev;
const struct pci_device_id *id;
+ struct work_struct work;
+ int error;
};
-static long local_pci_probe(void *_ddi)
+static void pci_probe_fn(struct work_struct *work)
{
- struct drv_dev_and_id *ddi = _ddi;
+ struct drv_dev_and_id *ddi = container_of(work, struct drv_dev_and_id,
+ work);
struct device *dev = &ddi->dev->dev;
int rc;
@@ -298,33 +301,38 @@ static long local_pci_probe(void *_ddi)
pm_runtime_set_suspended(dev);
pm_runtime_put_noidle(dev);
}
- return rc;
+
+ ddi->error = rc;
}
static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
const struct pci_device_id *id)
{
- int error, node;
+ int node;
+ int cpu = nr_cpu_ids;
struct drv_dev_and_id ddi = { drv, dev, id };
- /* Execute driver initialization on node where the device's
- bus is attached to. This way the driver likely allocates
- its local memory on the right node without any need to
- change it. */
- node = dev_to_node(&dev->dev);
- if (node >= 0) {
- int cpu;
+ /*
+ * Execute driver initialization on node where the device's bus is
+ * attached to. This way the driver likely allocates its local
+ * memory on the right node without any need to change it.
+ */
+ get_online_cpus();
- get_online_cpus();
+ node = dev_to_node(&dev->dev);
+ if (node >= 0)
cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
- if (cpu < nr_cpu_ids)
- error = work_on_cpu(cpu, local_pci_probe, &ddi);
- else
- error = local_pci_probe(&ddi);
- put_online_cpus();
- } else
- error = local_pci_probe(&ddi);
- return error;
+
+ if (cpu < nr_cpu_ids) {
+ INIT_WORK_ONSTACK(&ddi.work, pci_probe_fn);
+ schedule_work_on(cpu, &ddi.work);
+ flush_work(&ddi.work);
+ } else {
+ pci_probe_fn(&ddi.work);
+ }
+
+ put_online_cpus();
+ return ddi.error;
}
/**
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu()
2012-08-23 21:35 [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu() Tejun Heo
@ 2012-09-14 18:13 ` Bjorn Helgaas
2012-09-17 21:41 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2012-09-14 18:13 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-pci, linux-kernel
On Thu, Aug 23, 2012 at 3:35 PM, Tejun Heo <tj@kernel.org> wrote:
> pci_call_probe() uses work_on_cpu(), which creates and tears down a
> full kthread on each call, to invoke ->probe() on node local CPU for
> allocation affinity.
>
> The same goal can easily be achieved using a work item. This patch
> rewrites pci_call_probe() so that it uses a work item instead of
> work_on_cpu().
>
> Note that the function is restructured for simplicity. This adds
> get/put_online_cpus() pair for devices without node but the overhead
> of doing so isn't anything material at this level.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
I applied this to my "next" branch and pushed it. Thanks!
> ---
> I tested both paths but had to simulate device on foreign node.
>
> Thanks.
>
> drivers/pci/pci-driver.c | 48 +++++++++++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 5270f1a..554c15a 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -274,11 +274,14 @@ struct drv_dev_and_id {
> struct pci_driver *drv;
> struct pci_dev *dev;
> const struct pci_device_id *id;
> + struct work_struct work;
> + int error;
> };
>
> -static long local_pci_probe(void *_ddi)
> +static void pci_probe_fn(struct work_struct *work)
> {
> - struct drv_dev_and_id *ddi = _ddi;
> + struct drv_dev_and_id *ddi = container_of(work, struct drv_dev_and_id,
> + work);
> struct device *dev = &ddi->dev->dev;
> int rc;
>
> @@ -298,33 +301,38 @@ static long local_pci_probe(void *_ddi)
> pm_runtime_set_suspended(dev);
> pm_runtime_put_noidle(dev);
> }
> - return rc;
> +
> + ddi->error = rc;
> }
>
> static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
> const struct pci_device_id *id)
> {
> - int error, node;
> + int node;
> + int cpu = nr_cpu_ids;
> struct drv_dev_and_id ddi = { drv, dev, id };
>
> - /* Execute driver initialization on node where the device's
> - bus is attached to. This way the driver likely allocates
> - its local memory on the right node without any need to
> - change it. */
> - node = dev_to_node(&dev->dev);
> - if (node >= 0) {
> - int cpu;
> + /*
> + * Execute driver initialization on node where the device's bus is
> + * attached to. This way the driver likely allocates its local
> + * memory on the right node without any need to change it.
> + */
> + get_online_cpus();
>
> - get_online_cpus();
> + node = dev_to_node(&dev->dev);
> + if (node >= 0)
> cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
> - if (cpu < nr_cpu_ids)
> - error = work_on_cpu(cpu, local_pci_probe, &ddi);
> - else
> - error = local_pci_probe(&ddi);
> - put_online_cpus();
> - } else
> - error = local_pci_probe(&ddi);
> - return error;
> +
> + if (cpu < nr_cpu_ids) {
> + INIT_WORK_ONSTACK(&ddi.work, pci_probe_fn);
> + schedule_work_on(cpu, &ddi.work);
> + flush_work(&ddi.work);
> + } else {
> + pci_probe_fn(&ddi.work);
> + }
> +
> + put_online_cpus();
> + return ddi.error;
> }
>
> /**
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu()
2012-09-14 18:13 ` Bjorn Helgaas
@ 2012-09-17 21:41 ` Tejun Heo
2012-09-17 21:45 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2012-09-17 21:41 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel
On Fri, Sep 14, 2012 at 12:13:21PM -0600, Bjorn Helgaas wrote:
> On Thu, Aug 23, 2012 at 3:35 PM, Tejun Heo <tj@kernel.org> wrote:
> > pci_call_probe() uses work_on_cpu(), which creates and tears down a
> > full kthread on each call, to invoke ->probe() on node local CPU for
> > allocation affinity.
> >
> > The same goal can easily be achieved using a work item. This patch
> > rewrites pci_call_probe() so that it uses a work item instead of
> > work_on_cpu().
> >
> > Note that the function is restructured for simplicity. This adds
> > get/put_online_cpus() pair for devices without node but the overhead
> > of doing so isn't anything material at this level.
> >
> > Signed-off-by: Tejun Heo <tj@kernel.org>
>
> I applied this to my "next" branch and pushed it. Thanks!
Can you please drop / revert this one? It seems we need work_on_cpu()
after all and I'm gonna reimplement it in sane way.
Sorry about the trouble.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu()
2012-09-17 21:41 ` Tejun Heo
@ 2012-09-17 21:45 ` Bjorn Helgaas
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2012-09-17 21:45 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-pci, linux-kernel
On Mon, Sep 17, 2012 at 3:41 PM, Tejun Heo <tj@kernel.org> wrote:
> On Fri, Sep 14, 2012 at 12:13:21PM -0600, Bjorn Helgaas wrote:
>> On Thu, Aug 23, 2012 at 3:35 PM, Tejun Heo <tj@kernel.org> wrote:
>> > pci_call_probe() uses work_on_cpu(), which creates and tears down a
>> > full kthread on each call, to invoke ->probe() on node local CPU for
>> > allocation affinity.
>> >
>> > The same goal can easily be achieved using a work item. This patch
>> > rewrites pci_call_probe() so that it uses a work item instead of
>> > work_on_cpu().
>> >
>> > Note that the function is restructured for simplicity. This adds
>> > get/put_online_cpus() pair for devices without node but the overhead
>> > of doing so isn't anything material at this level.
>> >
>> > Signed-off-by: Tejun Heo <tj@kernel.org>
>>
>> I applied this to my "next" branch and pushed it. Thanks!
>
> Can you please drop / revert this one? It seems we need work_on_cpu()
> after all and I'm gonna reimplement it in sane way.
I dropped it. Thanks for the heads-up.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-17 21:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-23 21:35 [PATCH] pci: rewrite pci_call_probe() to use workqueue instead of work_on_cpu() Tejun Heo
2012-09-14 18:13 ` Bjorn Helgaas
2012-09-17 21:41 ` Tejun Heo
2012-09-17 21:45 ` Bjorn Helgaas
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).