* [PATCH v1 1/8] dmaengine: enhance DMA channel reference count management
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 2/8] dmaengine: rebalance DMA channels when CPU hotplug happens Jiang Liu
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
When hot-removing IOH (PCI host bridge) on Intel Nehalem/Westmere platform,
we need to remove all IOAT devices embedded in the IOH. For future Intel
processor with IIO (Embedded IOH), we need to remove IOAT devices even
when hot-removing a physical processor. But current dmaengine implementation
doesn't support hot-removal of IOAT devices at runtime.
Currently dmaengine has an assumption that a DMA device could only be
deregistered when there's no any clients making use of the dmaengine.
So dma_async_device_unregister() is designed to be called by DMA driver's
exit routines only. But the ioatdma driver doesn't conform to that design,
it calls dma_async_device_unregister() from dma_async_device_unregister(),
which is called in the driver detaching path.
This patch enhances dmaengine so dma_async_device_unregister() could be called
by DMA driver's detach routines when hot-removing DMA devices at runtime.
With currently implementation, variable dmaengine_ref_count is used to track
how many clients are making use of the dmaengine. There's also a per-channel
reference count named client_count in struct dma_chan. For successfully
initialized channels, dma_chan->client_count is set to dmaengine_ref_count.
That means all channels can't be released/removed if there are still clients.
So the solution here is to use dma_chan->client_count to track channel
reference count instead of tracking dmaengine_ref_count.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/dmaengine.c | 186 +++++++++++++++++------------------------------
1 files changed, 66 insertions(+), 120 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 2397f6f..5b6ad58 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -186,22 +186,6 @@ static struct module *dma_chan_to_owner(struct dma_chan *chan)
}
/**
- * balance_ref_count - catch up the channel reference count
- * @chan - channel to balance ->client_count versus dmaengine_ref_count
- *
- * balance_ref_count must be called under dma_list_mutex
- */
-static void balance_ref_count(struct dma_chan *chan)
-{
- struct module *owner = dma_chan_to_owner(chan);
-
- while (chan->client_count < dmaengine_ref_count) {
- __module_get(owner);
- chan->client_count++;
- }
-}
-
-/**
* dma_chan_get - try to grab a dma channel's parent driver module
* @chan - channel to grab
*
@@ -209,28 +193,24 @@ static void balance_ref_count(struct dma_chan *chan)
*/
static int dma_chan_get(struct dma_chan *chan)
{
- int err = -ENODEV;
+ int err = 0;
struct module *owner = dma_chan_to_owner(chan);
- if (chan->client_count) {
- __module_get(owner);
- err = 0;
- } else if (try_module_get(owner))
- err = 0;
+ /* Device driver module has been unloaded */
+ if (!try_module_get(owner))
+ return -ENODEV;
- if (err == 0)
- chan->client_count++;
+ chan->client_count++;
/* allocate upon first client reference */
- if (chan->client_count == 1 && err == 0) {
+ if (chan->client_count == 1) {
int desc_cnt = chan->device->device_alloc_chan_resources(chan);
if (desc_cnt < 0) {
err = desc_cnt;
chan->client_count = 0;
module_put(owner);
- } else if (!dma_has_cap(DMA_PRIVATE, chan->device->cap_mask))
- balance_ref_count(chan);
+ }
}
return err;
@@ -244,12 +224,11 @@ static int dma_chan_get(struct dma_chan *chan)
*/
static void dma_chan_put(struct dma_chan *chan)
{
- if (!chan->client_count)
- return; /* this channel failed alloc_chan_resources */
+ BUG_ON(chan->client_count <= 0);
chan->client_count--;
- module_put(dma_chan_to_owner(chan));
if (chan->client_count == 0)
chan->device->device_free_chan_resources(chan);
+ module_put(dma_chan_to_owner(chan));
}
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
@@ -278,9 +257,11 @@ static dma_cap_mask_t dma_cap_mask_all;
/**
* dma_chan_tbl_ent - tracks channel allocations per core/operation
* @chan - associated channel for this entry
+ * @prev_chan - previous associated channel for this entry
*/
struct dma_chan_tbl_ent {
struct dma_chan *chan;
+ struct dma_chan *prev_chan;
};
/**
@@ -367,7 +348,7 @@ void dma_issue_pending_all(void)
EXPORT_SYMBOL(dma_issue_pending_all);
/**
- * nth_chan - returns the nth channel of the given capability
+ * nth_chan - grab a reference to the nth channel of the given capability
* @cap: capability to match
* @n: nth channel desired
*
@@ -387,17 +368,19 @@ static struct dma_chan *nth_chan(enum dma_transaction_type cap, int n)
dma_has_cap(DMA_PRIVATE, device->cap_mask))
continue;
list_for_each_entry(chan, &device->channels, device_node) {
- if (!chan->client_count)
+ if (dma_chan_get(chan))
continue;
- if (!min)
- min = chan;
- else if (chan->table_count < min->table_count)
- min = chan;
-
if (n-- == 0) {
ret = chan;
break; /* done */
}
+ if (!min)
+ min = chan;
+ else if (chan->table_count < min->table_count) {
+ dma_chan_put(min);
+ min = chan;
+ } else
+ dma_chan_put(chan);
}
if (ret)
break; /* done */
@@ -405,6 +388,8 @@ static struct dma_chan *nth_chan(enum dma_transaction_type cap, int n)
if (!ret)
ret = min;
+ else if (min)
+ dma_chan_put(min);
if (ret)
ret->table_count++;
@@ -423,37 +408,39 @@ static struct dma_chan *nth_chan(enum dma_transaction_type cap, int n)
static void dma_channel_rebalance(void)
{
struct dma_chan *chan;
- struct dma_device *device;
+ struct dma_chan_tbl_ent *entry;
int cpu;
int cap;
- int n;
+ int n = 0;
- /* undo the last distribution */
+ /* save the last distribution */
for_each_dma_cap_mask(cap, dma_cap_mask_all)
- for_each_possible_cpu(cpu)
- per_cpu_ptr(channel_table[cap], cpu)->chan = NULL;
-
- list_for_each_entry(device, &dma_device_list, global_node) {
- if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
- continue;
- list_for_each_entry(chan, &device->channels, device_node)
- chan->table_count = 0;
- }
+ for_each_possible_cpu(cpu) {
+ entry = per_cpu_ptr(channel_table[cap], cpu);
+ entry->prev_chan = entry->chan;
+ entry->chan = NULL;
+ if (entry->prev_chan)
+ entry->prev_chan->table_count--;
+ }
- /* don't populate the channel_table if no clients are available */
- if (!dmaengine_ref_count)
- return;
+ /* redistribute available channels if there are clients */
+ if (dmaengine_ref_count)
+ for_each_dma_cap_mask(cap, dma_cap_mask_all)
+ for_each_online_cpu(cpu) {
+ if (num_possible_cpus() > 1)
+ chan = nth_chan(cap, n++);
+ else
+ chan = nth_chan(cap, -1);
+ entry = per_cpu_ptr(channel_table[cap], cpu);
+ entry->chan = chan;
+ }
- /* redistribute available channels */
- n = 0;
+ /* undo the last distribution */
for_each_dma_cap_mask(cap, dma_cap_mask_all)
- for_each_online_cpu(cpu) {
- if (num_possible_cpus() > 1)
- chan = nth_chan(cap, n++);
- else
- chan = nth_chan(cap, -1);
-
- per_cpu_ptr(channel_table[cap], cpu)->chan = chan;
+ for_each_possible_cpu(cpu) {
+ chan = per_cpu(channel_table[cap]->prev_chan, cpu);
+ if (chan)
+ dma_chan_put(chan);
}
}
@@ -511,19 +498,16 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v
chan = private_candidate(mask, device, fn, fn_param);
if (chan) {
/* Found a suitable channel, try to grab, prep, and
- * return it. We first set DMA_PRIVATE to disable
- * balance_ref_count as this channel will not be
- * published in the general-purpose allocator
+ * return it.
*/
dma_cap_set(DMA_PRIVATE, device->cap_mask);
device->privatecnt++;
err = dma_chan_get(chan);
- if (err == -ENODEV) {
+ if (err == -ENODEV)
pr_debug("%s: %s module removed\n", __func__,
dma_chan_name(chan));
- list_del_rcu(&device->global_node);
- } else if (err)
+ else if (err)
pr_debug("%s: failed to get %s: (%d)\n",
__func__, dma_chan_name(chan), err);
else
@@ -560,57 +544,26 @@ EXPORT_SYMBOL_GPL(dma_release_channel);
*/
void dmaengine_get(void)
{
- struct dma_device *device, *_d;
- struct dma_chan *chan;
- int err;
-
mutex_lock(&dma_list_mutex);
- dmaengine_ref_count++;
-
- /* try to grab channels */
- list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
- if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
- continue;
- list_for_each_entry(chan, &device->channels, device_node) {
- err = dma_chan_get(chan);
- if (err == -ENODEV) {
- /* module removed before we could use it */
- list_del_rcu(&device->global_node);
- break;
- } else if (err)
- pr_err("%s: failed to get %s: (%d)\n",
- __func__, dma_chan_name(chan), err);
- }
- }
-
/* if this is the first reference and there were channels
* waiting we need to rebalance to get those channels
* incorporated into the channel table
*/
- if (dmaengine_ref_count == 1)
+ if (++dmaengine_ref_count == 1)
dma_channel_rebalance();
mutex_unlock(&dma_list_mutex);
}
EXPORT_SYMBOL(dmaengine_get);
/**
- * dmaengine_put - let dma drivers be removed when ref_count == 0
+ * dmaengine_put - deregister interest in dma_channels
*/
void dmaengine_put(void)
{
- struct dma_device *device;
- struct dma_chan *chan;
-
mutex_lock(&dma_list_mutex);
- dmaengine_ref_count--;
- BUG_ON(dmaengine_ref_count < 0);
- /* drop channel references */
- list_for_each_entry(device, &dma_device_list, global_node) {
- if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
- continue;
- list_for_each_entry(chan, &device->channels, device_node)
- dma_chan_put(chan);
- }
+ BUG_ON(dmaengine_ref_count <= 0);
+ if (--dmaengine_ref_count == 0)
+ dma_channel_rebalance();
mutex_unlock(&dma_list_mutex);
}
EXPORT_SYMBOL(dmaengine_put);
@@ -773,26 +726,11 @@ int dma_async_device_register(struct dma_device *device)
device->chancnt = chancnt;
mutex_lock(&dma_list_mutex);
- /* take references on public channels */
- if (dmaengine_ref_count && !dma_has_cap(DMA_PRIVATE, device->cap_mask))
- list_for_each_entry(chan, &device->channels, device_node) {
- /* if clients are already waiting for channels we need
- * to take references on their behalf
- */
- if (dma_chan_get(chan) == -ENODEV) {
- /* note we can only get here for the first
- * channel as the remaining channels are
- * guaranteed to get a reference
- */
- rc = -ENODEV;
- mutex_unlock(&dma_list_mutex);
- goto err_out;
- }
- }
list_add_tail_rcu(&device->global_node, &dma_device_list);
if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
device->privatecnt++; /* Always private */
- dma_channel_rebalance();
+ else
+ dma_channel_rebalance();
mutex_unlock(&dma_list_mutex);
return 0;
@@ -836,6 +774,14 @@ void dma_async_device_unregister(struct dma_device *device)
dma_channel_rebalance();
mutex_unlock(&dma_list_mutex);
+ /* Check whether it's called from module exit function. */
+ if (try_module_get(device->dev->driver->owner)) {
+ dev_warn(device->dev,
+ "%s isn't called from module exit function.\n",
+ __func__);
+ module_put(device->dev->driver->owner);
+ }
+
list_for_each_entry(chan, &device->channels, device_node) {
WARN_ONCE(chan->client_count,
"%s called while %d clients hold a reference\n",
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 2/8] dmaengine: rebalance DMA channels when CPU hotplug happens
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
2012-04-23 13:51 ` [PATCH v1 1/8] dmaengine: enhance DMA channel reference count management Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 3/8] dmaengine: introduce CONFIG_DMA_ENGINE_HOTPLUG for DMA device hotplug Jiang Liu
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
Rebalance DMA channels when CPU hotplug happens to correctly update
DMA channel reference count.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/dmaengine.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 5b6ad58..198d891 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -49,6 +49,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/mm.h>
+#include <linux/cpu.h>
#include <linux/device.h>
#include <linux/dmaengine.h>
#include <linux/hardirq.h>
@@ -1008,8 +1009,29 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx)
}
EXPORT_SYMBOL_GPL(dma_run_dependencies);
+static int __cpuinit
+hotcpu_rebalance(struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+ switch (action) {
+ case CPU_ONLINE:
+ case CPU_POST_DEAD:
+ mutex_lock(&dma_list_mutex);
+ dma_channel_rebalance();
+ mutex_unlock(&dma_list_mutex);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block __refdata dma_cpu_notifier = {
+ .notifier_call = hotcpu_rebalance,
+};
+
+
static int __init dma_bus_init(void)
{
+ register_hotcpu_notifier(&dma_cpu_notifier);
return class_register(&dma_devclass);
}
arch_initcall(dma_bus_init);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 3/8] dmaengine: introduce CONFIG_DMA_ENGINE_HOTPLUG for DMA device hotplug
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
2012-04-23 13:51 ` [PATCH v1 1/8] dmaengine: enhance DMA channel reference count management Jiang Liu
2012-04-23 13:51 ` [PATCH v1 2/8] dmaengine: rebalance DMA channels when CPU hotplug happens Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 4/8] dmaengine: use atomic_t for struct dma_chan->client_count field Jiang Liu
` (5 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
Introduce CONFIG_DMA_ENGINE_HOTPLUG to enable/disable DMA device hotplug logic.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/Kconfig | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index cf9da36..aba92f0 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -62,6 +62,7 @@ config INTEL_IOATDMA
depends on PCI && X86
select DMA_ENGINE
select DCA
+ select DMA_ENGINE_HOTPLUG
select ASYNC_TX_DISABLE_PQ_VAL_DMA
select ASYNC_TX_DISABLE_XOR_VAL_DMA
help
@@ -263,6 +264,11 @@ config DMA_SA11X0
config DMA_ENGINE
bool
+config DMA_ENGINE_HOTPLUG
+ bool
+ default n
+ depends on DMA_ENGINE
+
comment "DMA Clients"
depends on DMA_ENGINE
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 4/8] dmaengine: use atomic_t for struct dma_chan->client_count field
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
` (2 preceding siblings ...)
2012-04-23 13:51 ` [PATCH v1 3/8] dmaengine: introduce CONFIG_DMA_ENGINE_HOTPLUG for DMA device hotplug Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 5/8] dmaengine: enhance dma_async_device_unregister() to be called by drv->remove() Jiang Liu
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
Use atomic_t for struct dma_chan->client_count field to prepare for
DMA device hotplug.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/dmaengine.c | 41 ++++++++++++++++++++++++++---------------
include/linux/dmaengine.h | 4 ++++
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 198d891..da7a683 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -62,6 +62,18 @@
#include <linux/idr.h>
#include <linux/slab.h>
+#ifndef CONFIG_DMA_ENGINE_HOTPLUG
+#define dma_chan_ref_read(chan) ((chan)->client_count)
+#define dma_chan_ref_set(chan, v) ((chan)->client_count = (v))
+#define dma_chan_ref_inc(chan) ((chan)->client_count++)
+#define dma_chan_ref_dec_and_test(ch) (--(chan)->client_count == 0)
+#else /* CONFIG_DMA_ENGINE_HOTPLUG */
+#define dma_chan_ref_read(chan) atomic_read(&(chan)->client_count)
+#define dma_chan_ref_set(chan, v) atomic_set(&(chan)->client_count, (v))
+#define dma_chan_ref_inc(chan) atomic_inc(&(chan)->client_count)
+#define dma_chan_ref_dec_and_test(ch) atomic_dec_and_test(&(ch)->client_count)
+#endif /* CONFIG_DMA_ENGINE_HOTPLUG */
+
static DEFINE_MUTEX(dma_list_mutex);
static DEFINE_IDR(dma_idr);
static LIST_HEAD(dma_device_list);
@@ -132,7 +144,7 @@ static ssize_t show_in_use(struct device *dev, struct device_attribute *attr, ch
mutex_lock(&dma_list_mutex);
chan = dev_to_dma_chan(dev);
if (chan)
- err = sprintf(buf, "%d\n", chan->client_count);
+ err = sprintf(buf, "%d\n", dma_chan_ref_read(chan));
else
err = -ENODEV;
mutex_unlock(&dma_list_mutex);
@@ -201,15 +213,15 @@ static int dma_chan_get(struct dma_chan *chan)
if (!try_module_get(owner))
return -ENODEV;
- chan->client_count++;
+ dma_chan_ref_inc(chan);
/* allocate upon first client reference */
- if (chan->client_count == 1) {
+ if (dma_chan_ref_read(chan) == 1) {
int desc_cnt = chan->device->device_alloc_chan_resources(chan);
if (desc_cnt < 0) {
err = desc_cnt;
- chan->client_count = 0;
+ dma_chan_ref_set(chan, 0);
module_put(owner);
}
}
@@ -225,9 +237,8 @@ static int dma_chan_get(struct dma_chan *chan)
*/
static void dma_chan_put(struct dma_chan *chan)
{
- BUG_ON(chan->client_count <= 0);
- chan->client_count--;
- if (chan->client_count == 0)
+ BUG_ON(dma_chan_ref_read(chan) <= 0);
+ if (dma_chan_ref_dec_and_test(chan))
chan->device->device_free_chan_resources(chan);
module_put(dma_chan_to_owner(chan));
}
@@ -341,7 +352,7 @@ void dma_issue_pending_all(void)
if (dma_has_cap(DMA_PRIVATE, device->cap_mask))
continue;
list_for_each_entry(chan, &device->channels, device_node)
- if (chan->client_count)
+ if (dma_chan_ref_read(chan))
device->device_issue_pending(chan);
}
rcu_read_unlock();
@@ -460,12 +471,12 @@ static struct dma_chan *private_candidate(dma_cap_mask_t *mask, struct dma_devic
if (dev->chancnt > 1 && !dma_has_cap(DMA_PRIVATE, dev->cap_mask))
list_for_each_entry(chan, &dev->channels, device_node) {
/* some channels are already publicly allocated */
- if (chan->client_count)
+ if (dma_chan_ref_read(chan))
return NULL;
}
list_for_each_entry(chan, &dev->channels, device_node) {
- if (chan->client_count) {
+ if (dma_chan_ref_read(chan)) {
pr_debug("%s: %s busy\n",
__func__, dma_chan_name(chan));
continue;
@@ -530,8 +541,8 @@ EXPORT_SYMBOL_GPL(__dma_request_channel);
void dma_release_channel(struct dma_chan *chan)
{
mutex_lock(&dma_list_mutex);
- WARN_ONCE(chan->client_count != 1,
- "chan reference count %d != 1\n", chan->client_count);
+ WARN_ONCE(dma_chan_ref_read(chan) != 1,
+ "chan reference count %d != 1\n", dma_chan_ref_read(chan));
dma_chan_put(chan);
/* drop PRIVATE cap enabled by __dma_request_channel() */
if (--chan->device->privatecnt == 0)
@@ -722,7 +733,7 @@ int dma_async_device_register(struct dma_device *device)
atomic_dec(idr_ref);
goto err_out;
}
- chan->client_count = 0;
+ dma_chan_ref_set(chan, 0);
}
device->chancnt = chancnt;
@@ -784,9 +795,9 @@ void dma_async_device_unregister(struct dma_device *device)
}
list_for_each_entry(chan, &device->channels, device_node) {
- WARN_ONCE(chan->client_count,
+ WARN_ONCE(dma_chan_ref_read(chan),
"%s called while %d clients hold a reference\n",
- __func__, chan->client_count);
+ __func__, dma_chan_ref_read(chan));
mutex_lock(&dma_list_mutex);
chan->dev->chan = NULL;
mutex_unlock(&dma_list_mutex);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index f9a2e5e..d1532dc 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -279,7 +279,11 @@ struct dma_chan {
struct list_head device_node;
struct dma_chan_percpu __percpu *local;
+#ifdef CONFIG_DMA_ENGINE_HOTPLUG
+ atomic_t client_count;
+#else
int client_count;
+#endif
int table_count;
void *private;
};
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 5/8] dmaengine: enhance dma_async_device_unregister() to be called by drv->remove()
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
` (3 preceding siblings ...)
2012-04-23 13:51 ` [PATCH v1 4/8] dmaengine: use atomic_t for struct dma_chan->client_count field Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug Jiang Liu
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
This patch enhances dma_async_device_unregister() to be called by DMA
driver's detaching routines. To achieve that, it enhances dma_find_channel()
and net_dma_find_channel() to hold a reference count on returned channel,
also introduce dma_get/put_channel() to update DMA channel reference count.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/dmaengine.c | 110 ++++++++++++++++++++++++++++++++++++++++-----
include/linux/dmaengine.h | 12 +++++
2 files changed, 111 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index da7a683..1cb91df 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -61,17 +61,28 @@
#include <linux/rculist.h>
#include <linux/idr.h>
#include <linux/slab.h>
+#include <linux/sched.h>
#ifndef CONFIG_DMA_ENGINE_HOTPLUG
#define dma_chan_ref_read(chan) ((chan)->client_count)
#define dma_chan_ref_set(chan, v) ((chan)->client_count = (v))
#define dma_chan_ref_inc(chan) ((chan)->client_count++)
#define dma_chan_ref_dec_and_test(ch) (--(chan)->client_count == 0)
+#define dma_chan_rcu_get(var) (var)
+#define dma_chan_rcu_set(var, ptr) ((var) = (ptr))
+#define dma_chan_rcu_access_ptr(var) (var)
+#define dma_chan_rcu_space
#else /* CONFIG_DMA_ENGINE_HOTPLUG */
#define dma_chan_ref_read(chan) atomic_read(&(chan)->client_count)
#define dma_chan_ref_set(chan, v) atomic_set(&(chan)->client_count, (v))
#define dma_chan_ref_inc(chan) atomic_inc(&(chan)->client_count)
#define dma_chan_ref_dec_and_test(ch) atomic_dec_and_test(&(ch)->client_count)
+#define dma_chan_rcu_get(var) rcu_dereference(var);
+#define dma_chan_rcu_set(var, ptr) rcu_assign_pointer((var), (ptr))
+#define dma_chan_rcu_access_ptr(var) rcu_access_pointer((var));
+#define dma_chan_rcu_space __rcu
+
+static DECLARE_WAIT_QUEUE_HEAD(dma_device_wait_queue);
#endif /* CONFIG_DMA_ENGINE_HOTPLUG */
static DEFINE_MUTEX(dma_list_mutex);
@@ -238,8 +249,12 @@ static int dma_chan_get(struct dma_chan *chan)
static void dma_chan_put(struct dma_chan *chan)
{
BUG_ON(dma_chan_ref_read(chan) <= 0);
- if (dma_chan_ref_dec_and_test(chan))
+ if (unlikely(dma_chan_ref_dec_and_test(chan))) {
chan->device->device_free_chan_resources(chan);
+#ifdef CONFIG_DMA_ENGINE_HOTPLUG
+ wake_up_all(&dma_device_wait_queue);
+#endif
+ }
module_put(dma_chan_to_owner(chan));
}
@@ -272,7 +287,7 @@ static dma_cap_mask_t dma_cap_mask_all;
* @prev_chan - previous associated channel for this entry
*/
struct dma_chan_tbl_ent {
- struct dma_chan *chan;
+ struct dma_chan dma_chan_rcu_space *chan;
struct dma_chan *prev_chan;
};
@@ -316,29 +331,86 @@ static int __init dma_channel_table_init(void)
arch_initcall(dma_channel_table_init);
/**
- * dma_find_channel - find a channel to carry out the operation
+ * dma_has_capability - check whether any channel supports tx_type
* @tx_type: transaction type
*/
-struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
+bool dma_has_capability(enum dma_transaction_type tx_type)
{
- return this_cpu_read(channel_table[tx_type]->chan);
+ struct dma_chan_tbl_ent *entry = this_cpu_ptr(channel_table[tx_type]);
+
+ return !!dma_chan_rcu_access_ptr(entry->chan);
}
-EXPORT_SYMBOL(dma_find_channel);
+EXPORT_SYMBOL(dma_has_capability);
/*
- * net_dma_find_channel - find a channel for net_dma
+ * net_dma_find_channel - find and hold a channel for net_dma
* net_dma has alignment requirements
*/
struct dma_chan *net_dma_find_channel(void)
{
struct dma_chan *chan = dma_find_channel(DMA_MEMCPY);
- if (chan && !is_dma_copy_aligned(chan->device, 1, 1, 1))
+
+ if (chan && !is_dma_copy_aligned(chan->device, 1, 1, 1)) {
+ dma_put_channel(chan);
return NULL;
+ }
return chan;
}
EXPORT_SYMBOL(net_dma_find_channel);
+#ifndef CONFIG_DMA_ENGINE_HOTPLUG
+/**
+ * dma_find_channel - find and get a channel to carry out the operation
+ * @tx_type: transaction type
+ */
+struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
+{
+ return this_cpu_read(channel_table[tx_type]->chan);
+}
+EXPORT_SYMBOL(dma_find_channel);
+
+#else /* CONFIG_DMA_ENGINE_HOTPLUG */
+
+/**
+ * dma_find_channel - find and get a channel to carry out the operation
+ * @tx_type: transaction type
+ */
+struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
+{
+ struct dma_chan_tbl_ent *entry = this_cpu_ptr(channel_table[tx_type]);
+ struct dma_chan *chan;
+
+ rcu_read_lock();
+ chan = rcu_dereference(entry->chan);
+ if (chan)
+ dma_chan_ref_inc(chan);
+ rcu_read_unlock();
+
+ return chan;
+}
+EXPORT_SYMBOL(dma_find_channel);
+
+struct dma_chan *dma_get_channel(struct dma_chan *chan)
+{
+ if (chan)
+ dma_chan_ref_inc(chan);
+
+ return chan;
+}
+EXPORT_SYMBOL(dma_get_channel);
+
+void dma_put_channel(struct dma_chan *chan)
+{
+ if (chan)
+ if (unlikely(dma_chan_ref_dec_and_test(chan))) {
+ chan->device->device_free_chan_resources(chan);
+ wake_up_all(&dma_device_wait_queue);
+ }
+}
+EXPORT_SYMBOL(dma_put_channel);
+#endif /* CONFIG_DMA_ENGINE_HOTPLUG */
+
/**
* dma_issue_pending_all - flush all pending operations across all channels
*/
@@ -429,8 +501,8 @@ static void dma_channel_rebalance(void)
for_each_dma_cap_mask(cap, dma_cap_mask_all)
for_each_possible_cpu(cpu) {
entry = per_cpu_ptr(channel_table[cap], cpu);
- entry->prev_chan = entry->chan;
- entry->chan = NULL;
+ entry->prev_chan = dma_chan_rcu_get(entry->chan);
+ dma_chan_rcu_set(entry->chan, NULL);
if (entry->prev_chan)
entry->prev_chan->table_count--;
}
@@ -444,9 +516,14 @@ static void dma_channel_rebalance(void)
else
chan = nth_chan(cap, -1);
entry = per_cpu_ptr(channel_table[cap], cpu);
- entry->chan = chan;
+ dma_chan_rcu_set(entry->chan, chan);
}
+#ifdef CONFIG_DMA_ENGINE_HOTPLUG
+ /* Synchronize with dma_find_get_channel() */
+ synchronize_rcu();
+#endif
+
/* undo the last distribution */
for_each_dma_cap_mask(cap, dma_cap_mask_all)
for_each_possible_cpu(cpu) {
@@ -788,9 +865,17 @@ void dma_async_device_unregister(struct dma_device *device)
/* Check whether it's called from module exit function. */
if (try_module_get(device->dev->driver->owner)) {
+#ifndef CONFIG_DMA_ENGINE_HOTPLUG
dev_warn(device->dev,
"%s isn't called from module exit function.\n",
__func__);
+#else
+ list_for_each_entry(chan, &device->channels, device_node) {
+ /* TODO: notify clients to release channels*/
+ wait_event(dma_device_wait_queue,
+ dma_chan_ref_read(chan) == 0);
+ }
+#endif
module_put(device->dev->driver->owner);
}
@@ -804,6 +889,9 @@ void dma_async_device_unregister(struct dma_device *device)
device_unregister(&chan->dev->device);
free_percpu(chan->local);
}
+
+ /* Synchronize with dma_issue_pending_all() */
+ synchronize_rcu();
}
EXPORT_SYMBOL(dma_async_device_unregister);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index d1532dc..874f8de 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -977,9 +977,21 @@ static inline void dma_release_channel(struct dma_chan *chan)
int dma_async_device_register(struct dma_device *device);
void dma_async_device_unregister(struct dma_device *device);
void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
+bool dma_has_capability(enum dma_transaction_type tx_type);
struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
struct dma_chan *net_dma_find_channel(void);
#define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
+#ifdef CONFIG_DMA_ENGINE_HOTPLUG
+struct dma_chan *dma_get_channel(struct dma_chan *chan);
+void dma_put_channel(struct dma_chan *chan);
+#else /* CONFIG_DMA_ENGINE_HOTPLUG */
+static inline struct dma_chan *dma_get_channel(struct dma_chan *chan)
+{
+ return chan;
+}
+
+static inline void dma_put_channel(struct dma_chan *chan) {}
+#endif /* CONFIG_DMA_ENGINE_HOTPLUG */
/* --- Helper iov-locking functions --- */
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
` (4 preceding siblings ...)
2012-04-23 13:51 ` [PATCH v1 5/8] dmaengine: enhance dma_async_device_unregister() to be called by drv->remove() Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 18:30 ` Dan Williams
2012-04-23 13:51 ` Jiang Liu
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
linux-pci, linux-kernel, Jiang Liu
Enhance network subsystem to correctly update DMA channel reference counts,
so it won't break DMA device hotplug logic.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
include/net/netdma.h | 26 ++++++++++++++++++++++++++
net/ipv4/tcp.c | 10 +++-------
net/ipv4/tcp_input.c | 5 +----
net/ipv4/tcp_ipv4.c | 4 +---
net/ipv6/tcp_ipv6.c | 4 +---
5 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/include/net/netdma.h b/include/net/netdma.h
index 8ba8ce2..6d71724 100644
--- a/include/net/netdma.h
+++ b/include/net/netdma.h
@@ -24,6 +24,32 @@
#include <linux/dmaengine.h>
#include <linux/skbuff.h>
+static inline bool
+net_dma_capable(void)
+{
+ struct dma_chan *chan = net_dma_find_channel();
+ dma_put_channel(chan);
+
+ return !!chan;
+}
+
+static inline struct dma_chan *
+net_dma_get_channel(struct tcp_sock *tp)
+{
+ if (!tp->ucopy.dma_chan && tp->ucopy.pinned_list)
+ tp->ucopy.dma_chan = net_dma_find_channel();
+ return tp->ucopy.dma_chan;
+}
+
+static inline void
+net_dma_put_channel(struct tcp_sock *tp)
+{
+ if (tp->ucopy.dma_chan) {
+ dma_put_channel(tp->ucopy.dma_chan);
+ tp->ucopy.dma_chan = NULL;
+ }
+}
+
int dma_skb_copy_datagram_iovec(struct dma_chan* chan,
struct sk_buff *skb, int offset, struct iovec *to,
size_t len, struct dma_pinned_list *pinned_list);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 8bb6ade..aea4032 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1451,8 +1451,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
available = TCP_SKB_CB(skb)->seq + skb->len - (*seq);
if ((available < target) &&
(len > sysctl_tcp_dma_copybreak) && !(flags & MSG_PEEK) &&
- !sysctl_tcp_low_latency &&
- net_dma_find_channel()) {
+ !sysctl_tcp_low_latency && net_dma_capable()) {
preempt_enable_no_resched();
tp->ucopy.pinned_list =
dma_pin_iovec_pages(msg->msg_iov, len);
@@ -1666,10 +1665,7 @@ do_prequeue:
if (!(flags & MSG_TRUNC)) {
#ifdef CONFIG_NET_DMA
- if (!tp->ucopy.dma_chan && tp->ucopy.pinned_list)
- tp->ucopy.dma_chan = net_dma_find_channel();
-
- if (tp->ucopy.dma_chan) {
+ if (net_dma_get_channel(tp)) {
tp->ucopy.dma_cookie = dma_skb_copy_datagram_iovec(
tp->ucopy.dma_chan, skb, offset,
msg->msg_iov, used,
@@ -1758,7 +1754,7 @@ skip_copy:
#ifdef CONFIG_NET_DMA
tcp_service_net_dma(sk, true); /* Wait for queue to drain */
- tp->ucopy.dma_chan = NULL;
+ net_dma_put_channel(tp);
if (tp->ucopy.pinned_list) {
dma_unpin_iovec_pages(tp->ucopy.pinned_list);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9944c1d..3878916 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5227,10 +5227,7 @@ static int tcp_dma_try_early_copy(struct sock *sk, struct sk_buff *skb,
if (tp->ucopy.wakeup)
return 0;
- if (!tp->ucopy.dma_chan && tp->ucopy.pinned_list)
- tp->ucopy.dma_chan = net_dma_find_channel();
-
- if (tp->ucopy.dma_chan && skb_csum_unnecessary(skb)) {
+ if (net_dma_get_channel(tp) && skb_csum_unnecessary(skb)) {
dma_cookie = dma_skb_copy_datagram_iovec(tp->ucopy.dma_chan,
skb, hlen,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0cb86ce..90ea1c0 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1729,9 +1729,7 @@ process:
if (!sock_owned_by_user(sk)) {
#ifdef CONFIG_NET_DMA
struct tcp_sock *tp = tcp_sk(sk);
- if (!tp->ucopy.dma_chan && tp->ucopy.pinned_list)
- tp->ucopy.dma_chan = net_dma_find_channel();
- if (tp->ucopy.dma_chan)
+ if (net_dma_get_channel(tp))
ret = tcp_v4_do_rcv(sk, skb);
else
#endif
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 86cfe60..fb81bbd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1644,9 +1644,7 @@ process:
if (!sock_owned_by_user(sk)) {
#ifdef CONFIG_NET_DMA
struct tcp_sock *tp = tcp_sk(sk);
- if (!tp->ucopy.dma_chan && tp->ucopy.pinned_list)
- tp->ucopy.dma_chan = net_dma_find_channel();
- if (tp->ucopy.dma_chan)
+ if (net_dma_get_channel(tp))
ret = tcp_v6_do_rcv(sk, skb);
else
#endif
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-23 13:51 ` [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug Jiang Liu
@ 2012-04-23 18:30 ` Dan Williams
2012-04-24 2:30 ` Jiang Liu
0 siblings, 1 reply; 17+ messages in thread
From: Dan Williams @ 2012-04-23 18:30 UTC (permalink / raw)
To: Jiang Liu
Cc: Vinod Koul, Jiang Liu, Keping Chen, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, linux-pci, linux-kernel
On Mon, Apr 23, 2012 at 6:51 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Enhance network subsystem to correctly update DMA channel reference counts,
> so it won't break DMA device hotplug logic.
>
> Signed-off-by: Jiang Liu <liuj97@gmail.com>
This introduces an atomic action on every channel touch, which is more
expensive than what we had previously. There has always been a
concern about the overhead of offload that sometimes makes ineffective
or a loss compared to cpu copies. In the cases where net_dma shows
improvement this will eat into / maybe eliminate that advantage.
Take a look at where dmaengine started [1]. It was from the beginning
going through contortions to avoid something like this. We made it
simpler here [2], but still kept the principle of not dirtying a
shared cacheline on every channel touch, and certainly not locking it.
If you are going to hotplug the entire IOH, then you are probably ok
with network links going down, so could you just down the links and
remove the driver with the existing code?
--
Dan
[1]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=c13c826
[2]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=6f49a57a
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-23 18:30 ` Dan Williams
@ 2012-04-24 2:30 ` Jiang Liu
2012-04-24 3:09 ` Dan Williams
0 siblings, 1 reply; 17+ messages in thread
From: Jiang Liu @ 2012-04-24 2:30 UTC (permalink / raw)
To: Dan Williams
Cc: Jiang Liu, Vinod Koul, Keping Chen, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, linux-pci, linux-kernel
Hi Dan,
Thanks for your great comments!
gerry
On 2012-4-24 2:30, Dan Williams wrote:
> On Mon, Apr 23, 2012 at 6:51 AM, Jiang Liu<liuj97@gmail.com> wrote:
>> Enhance network subsystem to correctly update DMA channel reference counts,
>> so it won't break DMA device hotplug logic.
>>
>> Signed-off-by: Jiang Liu<liuj97@gmail.com>
>
> This introduces an atomic action on every channel touch, which is more
> expensive than what we had previously. There has always been a
> concern about the overhead of offload that sometimes makes ineffective
> or a loss compared to cpu copies. In the cases where net_dma shows
> improvement this will eat into / maybe eliminate that advantage.
Good point, we should avoid pollute a shared cacheline here, otherwise
it may eat the benefits of IOAT acceleration.
>
> Take a look at where dmaengine started [1]. It was from the beginning
> going through contortions to avoid something like this. We made it
> simpler here [2], but still kept the principle of not dirtying a
> shared cacheline on every channel touch, and certainly not locking it.
Thanks for the great background information, especially the second one.
The check-in log message as below.
>Why?, beyond reducing complication:
>1/ Tracking reference counts per-transaction in an efficient manner, as
> is currently done, requires a complicated scheme to avoid cache-line
> bouncing effects.
The really issue here is polluting shared cachelines here, right?
Will it help to use percpu counter instead of atomic operations here?
I will have a try to use percpu counter for reference count.
BTW, do you have any DMAEngine benchmarks so we could use them to
compare the performance difference?
>2/ Per-transaction ref-counting gives the false impression that a
> dma-driver can be gracefully removed ahead of its user (net, md, or
> dma-slave)
>3/ None of the in-tree dma-drivers talk to hot pluggable hardware, but
Seems the situation has changed now:)
Intel 7500 (Boxboro) chipset supports hotplug. And we are working on
a system, which adopts Boxboro chipset and supports node hotplug.
So we try to enhance the DMAEngine to support IOAT hotplug.
On the other hand, Intel next generation processor Ivybridge has
embedded IOH, so we need to support IOH/IOAT hotplug when supporting
processor hotplug.
> if such an engine were built one day we still would not need to >notify
> clients of remove events. The driver can simply return NULL to a
> ->prep() request, something that is much easier for a client to
>handle.
Could you please help to give more explanations about "The driver can
simply return NULL to a ->prep() request", I have gotten the idea yet.
>
> If you are going to hotplug the entire IOH, then you are probably ok
> with network links going down, so could you just down the links and
> remove the driver with the existing code?
I feel it's a little risky to shut down/restart all network interfaces
for hot-removal of IOH, that may disturb the applications. And there
are also other kinds of clients, such as ASYNC_TX, seems we can't
adopt this method to reclaim DMA channels from ASYNC_TX subsystem.
>
> --
> Dan
>
> [1]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=c13c826
> [2]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=6f49a57a
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-24 2:30 ` Jiang Liu
@ 2012-04-24 3:09 ` Dan Williams
2012-04-24 3:56 ` Jiang Liu
2012-04-25 15:47 ` Jiang Liu
0 siblings, 2 replies; 17+ messages in thread
From: Dan Williams @ 2012-04-24 3:09 UTC (permalink / raw)
To: Jiang Liu
Cc: Jiang Liu, Vinod Koul, Keping Chen, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, linux-pci, linux-kernel
On Mon, Apr 23, 2012 at 7:30 PM, Jiang Liu <jiang.liu@huawei.com> wrote:
>> If you are going to hotplug the entire IOH, then you are probably ok
>> with network links going down, so could you just down the links and
>> remove the driver with the existing code?
>
> I feel it's a little risky to shut down/restart all network interfaces
> for hot-removal of IOH, that may disturb the applications.
I guess I'm confused... wouldn't the removal of an entire domain of
pci devices disturb userspace applications?
> And there
> are also other kinds of clients, such as ASYNC_TX, seems we can't
> adopt this method to reclaim DMA channels from ASYNC_TX subsystem.
I say handle this like block device hotplug. I.e. the driver stays
loaded but the channel is put into an 'offline' state. So the driver
hides the fact that the hardware went away. Similar to how you can
remove a disk but /dev/sda sticks around until the last reference is
gone (and the driver 'sd' sticks around until all block devices are
gone).
I expect the work will be in making sure existing clients are prepared
to handle NULL returns from ->device_prep_dma_*. In some cases the
channel is treated more like a cpu, so a NULL return from
->device_prep_dma_memcpy() has been interpreted as "device is
temporarily busy, it is safe to try again". We would need to change
that to a permanent indication that the device is gone and not attempt
retry.
--
Dan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-24 3:09 ` Dan Williams
@ 2012-04-24 3:56 ` Jiang Liu
2012-04-25 15:47 ` Jiang Liu
1 sibling, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-24 3:56 UTC (permalink / raw)
To: Dan Williams
Cc: Jiang Liu, Vinod Koul, Keping Chen, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, linux-pci, linux-kernel
On 2012-4-24 11:09, Dan Williams wrote:
> On Mon, Apr 23, 2012 at 7:30 PM, Jiang Liu<jiang.liu@huawei.com> wrote:
>>> If you are going to hotplug the entire IOH, then you are probably ok
>>> with network links going down, so could you just down the links and
>>> remove the driver with the existing code?
>>
>> I feel it's a little risky to shut down/restart all network interfaces
>> for hot-removal of IOH, that may disturb the applications.
>
> I guess I'm confused... wouldn't the removal of an entire domain of
> pci devices disturb userspace applications?
Here I mean removing an IOH shouldn't affect devices under other IOHs
if possible.
With current dmaengine implementation, a DMA device/channel may be used
by clients in other PCI domains. So to safely remove a DMA device, we
need to return dmaengine_ref_count to zero by stopping all DMA clients.
For network, that means we need to stop all network interfaces, seems
a little heavy:)
>
>> And there
>> are also other kinds of clients, such as ASYNC_TX, seems we can't
>> adopt this method to reclaim DMA channels from ASYNC_TX subsystem.
>
> I say handle this like block device hotplug. I.e. the driver stays
> loaded but the channel is put into an 'offline' state. So the driver
> hides the fact that the hardware went away. Similar to how you can
> remove a disk but /dev/sda sticks around until the last reference is
> gone (and the driver 'sd' sticks around until all block devices are
> gone).
Per my understanding, this mechanism could be used to stop driver from
accessing surprisingly removed devices, but it still needs a reference
count mechanism to finish the driver unbinding operation eventually.
For IOH hotplug, we need to wait for the completion of driver unbinding
operations before destroying the PCI device nodes of IOAT, so still need
reference count to track channel usage.
Another way is to notify all clients to release all channels when IOAT
device hotplug happens, but that may need heavy modification to the
DMA clients.
>
> I expect the work will be in making sure existing clients are prepared
> to handle NULL returns from ->device_prep_dma_*. In some cases the
> channel is treated more like a cpu, so a NULL return from
> ->device_prep_dma_memcpy() has been interpreted as "device is
> temporarily busy, it is safe to try again". We would need to change
> that to a permanent indication that the device is gone and not attempt
> retry.
Yes, some ASYNC_TX clients interpret NULL return as EBUSY and keep on
retry when doing context aware computations. Will try to investigate
on this direction.
>
> --
> Dan
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 6/8] dmaengine: enhance network subsystem to support DMA device hotplug
2012-04-24 3:09 ` Dan Williams
2012-04-24 3:56 ` Jiang Liu
@ 2012-04-25 15:47 ` Jiang Liu
1 sibling, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-25 15:47 UTC (permalink / raw)
To: Dan Williams
Cc: Jiang Liu, Vinod Koul, Keping Chen, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, linux-pci, linux-kernel
Hi Dan,
Thanks for your great comments about the performance penalty issue. And I'm trying
to refine the implementation to reduce penalty caused by hotplug logic. If the algorithm works
correctly, the optimized hot path code will be:
------------------------------------------------------------------------------
struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type)
{
struct dma_chan *chan = this_cpu_read(channel_table[tx_type]->chan);
this_cpu_inc(dmaengine_chan_ref_count);
if (static_key_false(&dmaengine_quiesce)) {
chan = NULL;
}
return chan;
}
EXPORT_SYMBOL(dma_find_channel);
struct dma_chan *dma_get_channel(struct dma_chan *chan)
{
if (static_key_false(&dmaengine_quiesce))
atomic_inc(&dmaengine_dirty);
this_cpu_inc(dmaengine_chan_ref_count);
return chan;
}
EXPORT_SYMBOL(dma_get_channel);
void dma_put_channel(struct dma_chan *chan)
{
this_cpu_dec(dmaengine_chan_ref_count);
}
EXPORT_SYMBOL(dma_put_channel);
-----------------------------------------------------------------------------
The disassembled code is:
(gdb) disassemble dma_find_channel
Dump of assembler code for function dma_find_channel:
0x0000000000000000 <+0>: push %rbp
0x0000000000000001 <+1>: mov %rsp,%rbp
0x0000000000000004 <+4>: callq 0x9 <dma_find_channel+9>
0x0000000000000009 <+9>: mov %edi,%edi
0x000000000000000b <+11>: mov 0x0(,%rdi,8),%rax
0x0000000000000013 <+19>: mov %gs:(%rax),%rax
0x0000000000000017 <+23>: incq %gs:0x0 //overhead: this_cpu_inc(dmaengine_chan_ref_count)
0x0000000000000020 <+32>: jmpq 0x25 <dma_find_channel+37> //overhead: if (static_key_false(&dmaengine_quiesce)), will be replaced as NOP by jump label
0x0000000000000025 <+37>: pop %rbp
0x0000000000000026 <+38>: retq
0x0000000000000027 <+39>: nopw 0x0(%rax,%rax,1)
0x0000000000000030 <+48>: xor %eax,%eax
0x0000000000000032 <+50>: pop %rbp
0x0000000000000033 <+51>: retq
End of assembler dump.
(gdb) disassemble dma_put_channel // overhead: to decrease channel reference count, 6 instructions
Dump of assembler code for function dma_put_channel:
0x0000000000000070 <+0>: push %rbp
0x0000000000000071 <+1>: mov %rsp,%rbp
0x0000000000000074 <+4>: callq 0x79 <dma_put_channel+9>
0x0000000000000079 <+9>: decq %gs:0x0
0x0000000000000082 <+18>: pop %rbp
0x0000000000000083 <+19>: retq
End of assembler dump.
(gdb) disassemble dma_get_channel
Dump of assembler code for function dma_get_channel:
0x0000000000000040 <+0>: push %rbp
0x0000000000000041 <+1>: mov %rsp,%rbp
0x0000000000000044 <+4>: callq 0x49 <dma_get_channel+9>
0x0000000000000049 <+9>: mov %rdi,%rax
0x000000000000004c <+12>: jmpq 0x51 <dma_get_channel+17>
0x0000000000000051 <+17>: incq %gs:0x0
0x000000000000005a <+26>: pop %rbp
0x000000000000005b <+27>: retq
0x000000000000005c <+28>: nopl 0x0(%rax)
0x0000000000000060 <+32>: lock incl 0x0(%rip) # 0x67 <dma_get_channel+39>
0x0000000000000067 <+39>: jmp 0x51 <dma_get_channel+17>
End of assembler dump.
So for a typical dma_find_channel()/dma_put_channel(), the total overhead
is about 10 instructions and two percpu(local) memory updates. And there's
no shared cache pollution any more. Is this acceptable ff the algorithm
works as expected? I will test the code tomorrow.
For typical systems which don't support DMA device hotplug, the overhead
could be completely removed by condition compilation.
Any comments are welcomed!
Thanks!
--gerry
On 04/24/2012 11:09 AM, Dan Williams wrote:
>>> If you are going to hotplug the entire IOH, then you are probably ok
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 7/8] dmaengine: enhance ASYNC_TX subsystem to support DMA device hotplug
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 13:51 ` [PATCH v1 2/8] dmaengine: rebalance DMA channels when CPU hotplug happens Jiang Liu
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, Herbert Xu, David S. Miller,
linux-crypto, linux-pci, linux-kernel, Jiang Liu
Enhance ASYNC_TX subsystem to correctly update DMA channel reference counts,
so it won't break DMA device hotplug logic.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
crypto/async_tx/async_memcpy.c | 2 ++
crypto/async_tx/async_memset.c | 2 ++
crypto/async_tx/async_pq.c | 10 ++++++++--
crypto/async_tx/async_raid6_recov.c | 8 ++++++--
crypto/async_tx/async_tx.c | 6 +++---
crypto/async_tx/async_xor.c | 13 +++++++++----
include/linux/async_tx.h | 13 +++++++++++++
include/linux/dmaengine.h | 14 ++++++++++++++
8 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c
index 361b5e8..0cbd90e 100644
--- a/crypto/async_tx/async_memcpy.c
+++ b/crypto/async_tx/async_memcpy.c
@@ -90,6 +90,8 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset,
async_tx_sync_epilog(submit);
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_memcpy);
diff --git a/crypto/async_tx/async_memset.c b/crypto/async_tx/async_memset.c
index 58e4a87..ec568bb 100644
--- a/crypto/async_tx/async_memset.c
+++ b/crypto/async_tx/async_memset.c
@@ -79,6 +79,8 @@ async_memset(struct page *dest, int val, unsigned int offset, size_t len,
async_tx_sync_epilog(submit);
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_memset);
diff --git a/crypto/async_tx/async_pq.c b/crypto/async_tx/async_pq.c
index 91d5d38..ae2070c 100644
--- a/crypto/async_tx/async_pq.c
+++ b/crypto/async_tx/async_pq.c
@@ -203,6 +203,7 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks,
blocks, src_cnt, len);
struct dma_device *device = chan ? chan->device : NULL;
dma_addr_t *dma_src = NULL;
+ struct dma_async_tx_descriptor *tx;
BUG_ON(disks > 255 || !(P(blocks, disks) || Q(blocks, disks)));
@@ -218,12 +219,15 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks,
/* run the p+q asynchronously */
pr_debug("%s: (async) disks: %d len: %zu\n",
__func__, disks, len);
- return do_async_gen_syndrome(chan, blocks, raid6_gfexp, offset,
- disks, len, dma_src, submit);
+ tx = do_async_gen_syndrome(chan, blocks, raid6_gfexp, offset,
+ disks, len, dma_src, submit);
+ async_tx_put_channel(chan);
+ return tx;
}
/* run the pq synchronously */
pr_debug("%s: (sync) disks: %d len: %zu\n", __func__, disks, len);
+ async_tx_put_channel(chan);
/* wait for any prerequisite operations */
async_tx_quiesce(&submit->depend_tx);
@@ -331,6 +335,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks,
dma_async_issue_pending(chan);
}
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
} else {
@@ -344,6 +349,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks,
pr_debug("%s: (sync) disks: %d len: %zu\n",
__func__, disks, len);
+ async_tx_put_channel(chan);
/* caller must provide a temporary result buffer and
* allow the input parameters to be preserved
diff --git a/crypto/async_tx/async_raid6_recov.c b/crypto/async_tx/async_raid6_recov.c
index a9f08a6..0f54d7c 100644
--- a/crypto/async_tx/async_raid6_recov.c
+++ b/crypto/async_tx/async_raid6_recov.c
@@ -54,6 +54,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef,
len, dma_flags);
if (tx) {
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
}
@@ -66,6 +67,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef,
}
/* run the operation synchronously */
+ async_tx_put_channel(chan);
async_tx_quiesce(&submit->depend_tx);
amul = raid6_gfmul[coef[0]];
bmul = raid6_gfmul[coef[1]];
@@ -107,6 +109,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len,
len, dma_flags);
if (tx) {
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
}
@@ -120,6 +123,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len,
/* no channel available, or failed to allocate a descriptor, so
* perform the operation synchronously
*/
+ async_tx_put_channel(chan);
async_tx_quiesce(&submit->depend_tx);
qmul = raid6_gfmul[coef];
d = page_address(dest);
@@ -339,7 +343,7 @@ async_raid6_2data_recov(int disks, size_t bytes, int faila, int failb,
* available' case be sure to use the scribble buffer to
* preserve the content of 'blocks' as the caller intended.
*/
- if (!async_dma_find_channel(DMA_PQ) || !scribble) {
+ if (!async_tx_has_capability(DMA_PQ) || !scribble) {
void **ptrs = scribble ? scribble : (void **) blocks;
async_tx_quiesce(&submit->depend_tx);
@@ -415,7 +419,7 @@ async_raid6_datap_recov(int disks, size_t bytes, int faila,
* available' case be sure to use the scribble buffer to
* preserve the content of 'blocks' as the caller intended.
*/
- if (!async_dma_find_channel(DMA_PQ) || !scribble) {
+ if (!async_tx_has_capability(DMA_PQ) || !scribble) {
void **ptrs = scribble ? scribble : (void **) blocks;
async_tx_quiesce(&submit->depend_tx);
diff --git a/crypto/async_tx/async_tx.c b/crypto/async_tx/async_tx.c
index 8421209..6fe6561 100644
--- a/crypto/async_tx/async_tx.c
+++ b/crypto/async_tx/async_tx.c
@@ -47,8 +47,8 @@ module_init(async_tx_init);
module_exit(async_tx_exit);
/**
- * __async_tx_find_channel - find a channel to carry out the operation or let
- * the transaction execute synchronously
+ * __async_tx_find_channel - find and hold a channel to carry out the
+ * operation or let the transaction execute synchronously
* @submit: transaction dependency and submission modifiers
* @tx_type: transaction type
*/
@@ -61,7 +61,7 @@ __async_tx_find_channel(struct async_submit_ctl *submit,
/* see if we can keep the chain on one channel */
if (depend_tx &&
dma_has_cap(tx_type, depend_tx->chan->device->cap_mask))
- return depend_tx->chan;
+ return async_dma_get_channel(depend_tx->chan);
return async_dma_find_channel(tx_type);
}
EXPORT_SYMBOL_GPL(__async_tx_find_channel);
diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c
index 154cc84..056e248 100644
--- a/crypto/async_tx/async_xor.c
+++ b/crypto/async_tx/async_xor.c
@@ -190,6 +190,7 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
&dest, 1, src_list,
src_cnt, len);
dma_addr_t *dma_src = NULL;
+ struct dma_async_tx_descriptor *tx = NULL;
BUG_ON(src_cnt <= 1);
@@ -202,8 +203,8 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
/* run the xor asynchronously */
pr_debug("%s (async): len: %zu\n", __func__, len);
- return do_async_xor(chan, dest, src_list, offset, src_cnt, len,
- dma_src, submit);
+ tx = do_async_xor(chan, dest, src_list, offset, src_cnt, len,
+ dma_src, submit);
} else {
/* run the xor synchronously */
pr_debug("%s (sync): len: %zu\n", __func__, len);
@@ -222,9 +223,11 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
async_tx_quiesce(&submit->depend_tx);
do_sync_xor(dest, src_list, offset, src_cnt, len, submit);
-
- return NULL;
}
+
+ async_tx_put_channel(chan);
+
+ return tx;
}
EXPORT_SYMBOL_GPL(async_xor);
@@ -330,6 +333,8 @@ async_xor_val(struct page *dest, struct page **src_list, unsigned int offset,
submit->flags = flags_orig;
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_xor_val);
diff --git a/include/linux/async_tx.h b/include/linux/async_tx.h
index a1c486a..35dea72 100644
--- a/include/linux/async_tx.h
+++ b/include/linux/async_tx.h
@@ -104,6 +104,10 @@ static inline void async_tx_issue_pending(struct dma_async_tx_descriptor *tx)
dma->device_issue_pending(chan);
}
}
+
+#define async_tx_put_channel(c) async_dma_put_channel(c)
+#define async_tx_has_capability(c) async_dma_has_capability(c)
+
#ifdef CONFIG_ARCH_HAS_ASYNC_TX_FIND_CHANNEL
#include <asm/async_tx.h>
#else
@@ -132,6 +136,15 @@ async_tx_find_channel(struct async_submit_ctl *submit,
{
return NULL;
}
+
+static inline void async_tx_put_channel(struct dma_chan *chan)
+{
+}
+
+static inline bool async_tx_has_capability(enum dma_transaction_type type)
+{
+ return false;
+}
#endif
/**
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 874f8de..99762a2 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -789,6 +789,9 @@ static inline void net_dmaengine_put(void)
#else
#define async_dma_find_channel(type) dma_find_channel(type)
#endif /* CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH */
+#define async_dma_get_channel(chan) dma_get_channel(chan)
+#define async_dma_put_channel(chan) dma_put_channel(chan)
+#define async_dma_has_capability(c) dma_has_capability(c)
#else
static inline void async_dmaengine_get(void)
{
@@ -801,6 +804,17 @@ async_dma_find_channel(enum dma_transaction_type type)
{
return NULL;
}
+static inline struct dma_chan *async_dma_get_channel(struct dma_chan *chan)
+{
+ return chan;
+}
+static inline void async_dma_put_channel(struct dma_chan *chan)
+{
+}
+static inline bool async_dma_has_capability(enum dma_transaction_type type)
+{
+ return false;
+}
#endif /* CONFIG_ASYNC_TX_DMA */
dma_cookie_t dma_async_memcpy_buf_to_buf(struct dma_chan *chan,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v1 7/8] dmaengine: enhance ASYNC_TX subsystem to support DMA device hotplug
@ 2012-04-23 13:51 ` Jiang Liu
0 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, Herbert Xu, David S. Miller,
linux-crypto, linux-pci, linux-kernel, Jiang Liu
Enhance ASYNC_TX subsystem to correctly update DMA channel reference counts,
so it won't break DMA device hotplug logic.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
crypto/async_tx/async_memcpy.c | 2 ++
crypto/async_tx/async_memset.c | 2 ++
crypto/async_tx/async_pq.c | 10 ++++++++--
crypto/async_tx/async_raid6_recov.c | 8 ++++++--
crypto/async_tx/async_tx.c | 6 +++---
crypto/async_tx/async_xor.c | 13 +++++++++----
include/linux/async_tx.h | 13 +++++++++++++
include/linux/dmaengine.h | 14 ++++++++++++++
8 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c
index 361b5e8..0cbd90e 100644
--- a/crypto/async_tx/async_memcpy.c
+++ b/crypto/async_tx/async_memcpy.c
@@ -90,6 +90,8 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset,
async_tx_sync_epilog(submit);
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_memcpy);
diff --git a/crypto/async_tx/async_memset.c b/crypto/async_tx/async_memset.c
index 58e4a87..ec568bb 100644
--- a/crypto/async_tx/async_memset.c
+++ b/crypto/async_tx/async_memset.c
@@ -79,6 +79,8 @@ async_memset(struct page *dest, int val, unsigned int offset, size_t len,
async_tx_sync_epilog(submit);
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_memset);
diff --git a/crypto/async_tx/async_pq.c b/crypto/async_tx/async_pq.c
index 91d5d38..ae2070c 100644
--- a/crypto/async_tx/async_pq.c
+++ b/crypto/async_tx/async_pq.c
@@ -203,6 +203,7 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks,
blocks, src_cnt, len);
struct dma_device *device = chan ? chan->device : NULL;
dma_addr_t *dma_src = NULL;
+ struct dma_async_tx_descriptor *tx;
BUG_ON(disks > 255 || !(P(blocks, disks) || Q(blocks, disks)));
@@ -218,12 +219,15 @@ async_gen_syndrome(struct page **blocks, unsigned int offset, int disks,
/* run the p+q asynchronously */
pr_debug("%s: (async) disks: %d len: %zu\n",
__func__, disks, len);
- return do_async_gen_syndrome(chan, blocks, raid6_gfexp, offset,
- disks, len, dma_src, submit);
+ tx = do_async_gen_syndrome(chan, blocks, raid6_gfexp, offset,
+ disks, len, dma_src, submit);
+ async_tx_put_channel(chan);
+ return tx;
}
/* run the pq synchronously */
pr_debug("%s: (sync) disks: %d len: %zu\n", __func__, disks, len);
+ async_tx_put_channel(chan);
/* wait for any prerequisite operations */
async_tx_quiesce(&submit->depend_tx);
@@ -331,6 +335,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks,
dma_async_issue_pending(chan);
}
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
} else {
@@ -344,6 +349,7 @@ async_syndrome_val(struct page **blocks, unsigned int offset, int disks,
pr_debug("%s: (sync) disks: %d len: %zu\n",
__func__, disks, len);
+ async_tx_put_channel(chan);
/* caller must provide a temporary result buffer and
* allow the input parameters to be preserved
diff --git a/crypto/async_tx/async_raid6_recov.c b/crypto/async_tx/async_raid6_recov.c
index a9f08a6..0f54d7c 100644
--- a/crypto/async_tx/async_raid6_recov.c
+++ b/crypto/async_tx/async_raid6_recov.c
@@ -54,6 +54,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef,
len, dma_flags);
if (tx) {
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
}
@@ -66,6 +67,7 @@ async_sum_product(struct page *dest, struct page **srcs, unsigned char *coef,
}
/* run the operation synchronously */
+ async_tx_put_channel(chan);
async_tx_quiesce(&submit->depend_tx);
amul = raid6_gfmul[coef[0]];
bmul = raid6_gfmul[coef[1]];
@@ -107,6 +109,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len,
len, dma_flags);
if (tx) {
async_tx_submit(chan, tx, submit);
+ async_tx_put_channel(chan);
return tx;
}
@@ -120,6 +123,7 @@ async_mult(struct page *dest, struct page *src, u8 coef, size_t len,
/* no channel available, or failed to allocate a descriptor, so
* perform the operation synchronously
*/
+ async_tx_put_channel(chan);
async_tx_quiesce(&submit->depend_tx);
qmul = raid6_gfmul[coef];
d = page_address(dest);
@@ -339,7 +343,7 @@ async_raid6_2data_recov(int disks, size_t bytes, int faila, int failb,
* available' case be sure to use the scribble buffer to
* preserve the content of 'blocks' as the caller intended.
*/
- if (!async_dma_find_channel(DMA_PQ) || !scribble) {
+ if (!async_tx_has_capability(DMA_PQ) || !scribble) {
void **ptrs = scribble ? scribble : (void **) blocks;
async_tx_quiesce(&submit->depend_tx);
@@ -415,7 +419,7 @@ async_raid6_datap_recov(int disks, size_t bytes, int faila,
* available' case be sure to use the scribble buffer to
* preserve the content of 'blocks' as the caller intended.
*/
- if (!async_dma_find_channel(DMA_PQ) || !scribble) {
+ if (!async_tx_has_capability(DMA_PQ) || !scribble) {
void **ptrs = scribble ? scribble : (void **) blocks;
async_tx_quiesce(&submit->depend_tx);
diff --git a/crypto/async_tx/async_tx.c b/crypto/async_tx/async_tx.c
index 8421209..6fe6561 100644
--- a/crypto/async_tx/async_tx.c
+++ b/crypto/async_tx/async_tx.c
@@ -47,8 +47,8 @@ module_init(async_tx_init);
module_exit(async_tx_exit);
/**
- * __async_tx_find_channel - find a channel to carry out the operation or let
- * the transaction execute synchronously
+ * __async_tx_find_channel - find and hold a channel to carry out the
+ * operation or let the transaction execute synchronously
* @submit: transaction dependency and submission modifiers
* @tx_type: transaction type
*/
@@ -61,7 +61,7 @@ __async_tx_find_channel(struct async_submit_ctl *submit,
/* see if we can keep the chain on one channel */
if (depend_tx &&
dma_has_cap(tx_type, depend_tx->chan->device->cap_mask))
- return depend_tx->chan;
+ return async_dma_get_channel(depend_tx->chan);
return async_dma_find_channel(tx_type);
}
EXPORT_SYMBOL_GPL(__async_tx_find_channel);
diff --git a/crypto/async_tx/async_xor.c b/crypto/async_tx/async_xor.c
index 154cc84..056e248 100644
--- a/crypto/async_tx/async_xor.c
+++ b/crypto/async_tx/async_xor.c
@@ -190,6 +190,7 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
&dest, 1, src_list,
src_cnt, len);
dma_addr_t *dma_src = NULL;
+ struct dma_async_tx_descriptor *tx = NULL;
BUG_ON(src_cnt <= 1);
@@ -202,8 +203,8 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
/* run the xor asynchronously */
pr_debug("%s (async): len: %zu\n", __func__, len);
- return do_async_xor(chan, dest, src_list, offset, src_cnt, len,
- dma_src, submit);
+ tx = do_async_xor(chan, dest, src_list, offset, src_cnt, len,
+ dma_src, submit);
} else {
/* run the xor synchronously */
pr_debug("%s (sync): len: %zu\n", __func__, len);
@@ -222,9 +223,11 @@ async_xor(struct page *dest, struct page **src_list, unsigned int offset,
async_tx_quiesce(&submit->depend_tx);
do_sync_xor(dest, src_list, offset, src_cnt, len, submit);
-
- return NULL;
}
+
+ async_tx_put_channel(chan);
+
+ return tx;
}
EXPORT_SYMBOL_GPL(async_xor);
@@ -330,6 +333,8 @@ async_xor_val(struct page *dest, struct page **src_list, unsigned int offset,
submit->flags = flags_orig;
}
+ async_tx_put_channel(chan);
+
return tx;
}
EXPORT_SYMBOL_GPL(async_xor_val);
diff --git a/include/linux/async_tx.h b/include/linux/async_tx.h
index a1c486a..35dea72 100644
--- a/include/linux/async_tx.h
+++ b/include/linux/async_tx.h
@@ -104,6 +104,10 @@ static inline void async_tx_issue_pending(struct dma_async_tx_descriptor *tx)
dma->device_issue_pending(chan);
}
}
+
+#define async_tx_put_channel(c) async_dma_put_channel(c)
+#define async_tx_has_capability(c) async_dma_has_capability(c)
+
#ifdef CONFIG_ARCH_HAS_ASYNC_TX_FIND_CHANNEL
#include <asm/async_tx.h>
#else
@@ -132,6 +136,15 @@ async_tx_find_channel(struct async_submit_ctl *submit,
{
return NULL;
}
+
+static inline void async_tx_put_channel(struct dma_chan *chan)
+{
+}
+
+static inline bool async_tx_has_capability(enum dma_transaction_type type)
+{
+ return false;
+}
#endif
/**
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 874f8de..99762a2 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -789,6 +789,9 @@ static inline void net_dmaengine_put(void)
#else
#define async_dma_find_channel(type) dma_find_channel(type)
#endif /* CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH */
+#define async_dma_get_channel(chan) dma_get_channel(chan)
+#define async_dma_put_channel(chan) dma_put_channel(chan)
+#define async_dma_has_capability(c) dma_has_capability(c)
#else
static inline void async_dmaengine_get(void)
{
@@ -801,6 +804,17 @@ async_dma_find_channel(enum dma_transaction_type type)
{
return NULL;
}
+static inline struct dma_chan *async_dma_get_channel(struct dma_chan *chan)
+{
+ return chan;
+}
+static inline void async_dma_put_channel(struct dma_chan *chan)
+{
+}
+static inline bool async_dma_has_capability(enum dma_transaction_type type)
+{
+ return false;
+}
#endif /* CONFIG_ASYNC_TX_DMA */
dma_cookie_t dma_async_memcpy_buf_to_buf(struct dma_chan *chan,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH v1 8/8] dmaengine: assign DMA channel to CPU according to NUMA affinity
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
` (6 preceding siblings ...)
2012-04-23 13:51 ` Jiang Liu
@ 2012-04-23 13:51 ` Jiang Liu
2012-04-23 16:40 ` [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Dan Williams
8 siblings, 0 replies; 17+ messages in thread
From: Jiang Liu @ 2012-04-23 13:51 UTC (permalink / raw)
To: Vinod Koul, Dan Williams
Cc: Jiang Liu, Keping Chen, linux-pci, linux-kernel, Jiang Liu
On systems with multiple CPUs and DMA devices, try optimize DMA
performance by assigning DMA channel to CPU according to NUMA
affinity relationship. This may help architectures with memory
controllers and DMA devices built into physical processors.
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
drivers/dma/dmaengine.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 1cb91df..52d748c 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -289,6 +289,7 @@ static dma_cap_mask_t dma_cap_mask_all;
struct dma_chan_tbl_ent {
struct dma_chan dma_chan_rcu_space *chan;
struct dma_chan *prev_chan;
+ int node;
};
/**
@@ -481,6 +482,46 @@ static struct dma_chan *nth_chan(enum dma_transaction_type cap, int n)
return ret;
}
+/* Assign DMA channels to CPUs according to NUMA affinity relationship */
+static void dma_channel_set(int cap, int cpu, struct dma_chan *chan)
+{
+ int node;
+ int src_cpu;
+ struct dma_chan *src_chan;
+ struct dma_chan_tbl_ent *entry;
+ struct dma_chan_tbl_ent *src_entry;
+
+ entry = per_cpu_ptr(channel_table[cap], cpu);
+ node = dev_to_node(chan->device->dev);
+
+ /* Try to optimize if CPU and DMA channel belong to different node. */
+ if (node != -1 && node != cpu_to_node(cpu)) {
+ for_each_online_cpu(src_cpu) {
+ src_entry = per_cpu_ptr(channel_table[cap], src_cpu);
+ src_chan = dma_chan_rcu_get(src_entry->chan);
+
+ /*
+ * CPU online map may change beneath us due to
+ * CPU hotplug operations.
+ */
+ if (src_chan == NULL)
+ continue;
+
+ if (src_entry->node == node ||
+ cpu_to_node(src_cpu) == node) {
+ entry->node = src_entry->node;
+ src_entry->node = node;
+ dma_chan_rcu_set(entry->chan, src_chan);
+ dma_chan_rcu_set(src_entry->chan, chan);
+ return;
+ }
+ }
+ }
+
+ entry->node = node;
+ dma_chan_rcu_set(entry->chan, chan);
+}
+
/**
* dma_channel_rebalance - redistribute the available channels
*
@@ -515,8 +556,8 @@ static void dma_channel_rebalance(void)
chan = nth_chan(cap, n++);
else
chan = nth_chan(cap, -1);
- entry = per_cpu_ptr(channel_table[cap], cpu);
- dma_chan_rcu_set(entry->chan, chan);
+ if (chan)
+ dma_channel_set(cap, cpu, chan);
}
#ifdef CONFIG_DMA_ENGINE_HOTPLUG
--
1.7.5.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug
2012-04-23 13:51 [PATCH v1 0/8] enhance dmaengine core to support DMA device hotplug Jiang Liu
` (7 preceding siblings ...)
2012-04-23 13:51 ` [RFC PATCH v1 8/8] dmaengine: assign DMA channel to CPU according to NUMA affinity Jiang Liu
@ 2012-04-23 16:40 ` Dan Williams
8 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2012-04-23 16:40 UTC (permalink / raw)
To: Jiang Liu; +Cc: Vinod Koul, Keping Chen, linux-pci, linux-kernel
On Mon, Apr 23, 2012 at 6:51 AM, Jiang Liu <liuj97@gmail.com> wrote:
> From: Jiang Liu <liuj97@gmail.com>
>
> This patchset tries to enhance the dmaengine and its clients to support
> hot-removal of DMA devices at runtime, especially for IOAT devices.
>
> When hot-removing IOH (PCI host bridge) on Intel Nehalem/Westmere platform,
> we need to remove all IOAT devices embedded in the IOH. For future Intel
> processor with IIO (Embedded IOH), we need to remove IOAT devices even
> when hot-removing a physical processor. But current dmaengine implementation
> doesn't support hot-removal of IOAT devices at runtime.
Removal of the host bridge means several devices disappear at once.
Is it safe to assume that this action is coordinated by userspace?
I.e. is the kernel given a chance to shut everything down and remove
drivers, or is this a "surprise" unplug?
--
Dan
^ permalink raw reply [flat|nested] 17+ messages in thread