dma debug: account for cachelines and read-only mappings in overlap tracking

While debug_dma_assert_idle() checks if a given *page* is actively
undergoing dma the valid granularity of a dma mapping is a *cacheline*.
Sander's testing shows that the warning message "DMA-API: exceeded 7
overlapping mappings of pfn..." is falsely triggering.  The test is
simply mapping multiple cachelines in a given page.

Ultimately we want overlap tracking to be valid as it is a real api
violation, so we need to track active mappings by cachelines.  Update
the active dma tracking to use the page-frame-relative cacheline of the
mapping as the key, and update debug_dma_assert_idle() to check for all
possible mapped cachelines for a given page.

However, the need to track active mappings is only relevant when the
dma-mapping is writable by the device.  In fact it is fairly standard
for read-only mappings to have hundreds or thousands of overlapping
mappings at once.  Limiting the overlap tracking to writable
(!DMA_TO_DEVICE) eliminates this class of false-positive overlap
reports.

Note, the radix gang lookup is sub-optimal.  It would be best if it
stopped fetching entries once the search passed a page boundary.
Nevertheless, this implementation does not perturb the original net_dma
failing case.  That is to say the extra overhead does not show up in
terms of making the failing case pass due to a timing change.

References:
  http://marc.info/?l=linux-netdev&m=139232263419315&w=2
  http://marc.info/?l=linux-netdev&m=139217088107122&w=2

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Reported-by: Dave Jones <davej@redhat.com>
Tested-by: Dave Jones <davej@redhat.com>
Tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Dan Williams 2014-03-03 15:38:21 -08:00 committed by Linus Torvalds
parent 668f9abbd4
commit 3b7a6418c7

View File

@ -424,111 +424,134 @@ void debug_dma_dump_mappings(struct device *dev)
EXPORT_SYMBOL(debug_dma_dump_mappings); EXPORT_SYMBOL(debug_dma_dump_mappings);
/* /*
* For each page mapped (initial page in the case of * For each mapping (initial cacheline in the case of
* dma_alloc_coherent/dma_map_{single|page}, or each page in a * dma_alloc_coherent/dma_map_page, initial cacheline in each page of a
* scatterlist) insert into this tree using the pfn as the key. At * scatterlist, or the cacheline specified in dma_map_single) insert
* into this tree using the cacheline as the key. At
* dma_unmap_{single|sg|page} or dma_free_coherent delete the entry. If * dma_unmap_{single|sg|page} or dma_free_coherent delete the entry. If
* the pfn already exists at insertion time add a tag as a reference * the entry already exists at insertion time add a tag as a reference
* count for the overlapping mappings. For now, the overlap tracking * count for the overlapping mappings. For now, the overlap tracking
* just ensures that 'unmaps' balance 'maps' before marking the pfn * just ensures that 'unmaps' balance 'maps' before marking the
* idle, but we should also be flagging overlaps as an API violation. * cacheline idle, but we should also be flagging overlaps as an API
* violation.
* *
* Memory usage is mostly constrained by the maximum number of available * Memory usage is mostly constrained by the maximum number of available
* dma-debug entries in that we need a free dma_debug_entry before * dma-debug entries in that we need a free dma_debug_entry before
* inserting into the tree. In the case of dma_map_{single|page} and * inserting into the tree. In the case of dma_map_page and
* dma_alloc_coherent there is only one dma_debug_entry and one pfn to * dma_alloc_coherent there is only one dma_debug_entry and one
* track per event. dma_map_sg(), on the other hand, * dma_active_cacheline entry to track per event. dma_map_sg(), on the
* consumes a single dma_debug_entry, but inserts 'nents' entries into * other hand, consumes a single dma_debug_entry, but inserts 'nents'
* the tree. * entries into the tree.
* *
* At any time debug_dma_assert_idle() can be called to trigger a * At any time debug_dma_assert_idle() can be called to trigger a
* warning if the given page is in the active set. * warning if any cachelines in the given page are in the active set.
*/ */
static RADIX_TREE(dma_active_pfn, GFP_NOWAIT); static RADIX_TREE(dma_active_cacheline, GFP_NOWAIT);
static DEFINE_SPINLOCK(radix_lock); static DEFINE_SPINLOCK(radix_lock);
#define ACTIVE_PFN_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1) #define ACTIVE_CACHELINE_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
#define CACHELINE_PER_PAGE_SHIFT (PAGE_SHIFT - L1_CACHE_SHIFT)
#define CACHELINES_PER_PAGE (1 << CACHELINE_PER_PAGE_SHIFT)
static int active_pfn_read_overlap(unsigned long pfn) static phys_addr_t to_cacheline_number(struct dma_debug_entry *entry)
{
return (entry->pfn << CACHELINE_PER_PAGE_SHIFT) +
(entry->offset >> L1_CACHE_SHIFT);
}
static int active_cacheline_read_overlap(phys_addr_t cln)
{ {
int overlap = 0, i; int overlap = 0, i;
for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--) for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--)
if (radix_tree_tag_get(&dma_active_pfn, pfn, i)) if (radix_tree_tag_get(&dma_active_cacheline, cln, i))
overlap |= 1 << i; overlap |= 1 << i;
return overlap; return overlap;
} }
static int active_pfn_set_overlap(unsigned long pfn, int overlap) static int active_cacheline_set_overlap(phys_addr_t cln, int overlap)
{ {
int i; int i;
if (overlap > ACTIVE_PFN_MAX_OVERLAP || overlap < 0) if (overlap > ACTIVE_CACHELINE_MAX_OVERLAP || overlap < 0)
return overlap; return overlap;
for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--) for (i = RADIX_TREE_MAX_TAGS - 1; i >= 0; i--)
if (overlap & 1 << i) if (overlap & 1 << i)
radix_tree_tag_set(&dma_active_pfn, pfn, i); radix_tree_tag_set(&dma_active_cacheline, cln, i);
else else
radix_tree_tag_clear(&dma_active_pfn, pfn, i); radix_tree_tag_clear(&dma_active_cacheline, cln, i);
return overlap; return overlap;
} }
static void active_pfn_inc_overlap(unsigned long pfn) static void active_cacheline_inc_overlap(phys_addr_t cln)
{ {
int overlap = active_pfn_read_overlap(pfn); int overlap = active_cacheline_read_overlap(cln);
overlap = active_pfn_set_overlap(pfn, ++overlap); overlap = active_cacheline_set_overlap(cln, ++overlap);
/* If we overflowed the overlap counter then we're potentially /* If we overflowed the overlap counter then we're potentially
* leaking dma-mappings. Otherwise, if maps and unmaps are * leaking dma-mappings. Otherwise, if maps and unmaps are
* balanced then this overflow may cause false negatives in * balanced then this overflow may cause false negatives in
* debug_dma_assert_idle() as the pfn may be marked idle * debug_dma_assert_idle() as the cacheline may be marked idle
* prematurely. * prematurely.
*/ */
WARN_ONCE(overlap > ACTIVE_PFN_MAX_OVERLAP, WARN_ONCE(overlap > ACTIVE_CACHELINE_MAX_OVERLAP,
"DMA-API: exceeded %d overlapping mappings of pfn %lx\n", "DMA-API: exceeded %d overlapping mappings of cacheline %pa\n",
ACTIVE_PFN_MAX_OVERLAP, pfn); ACTIVE_CACHELINE_MAX_OVERLAP, &cln);
} }
static int active_pfn_dec_overlap(unsigned long pfn) static int active_cacheline_dec_overlap(phys_addr_t cln)
{ {
int overlap = active_pfn_read_overlap(pfn); int overlap = active_cacheline_read_overlap(cln);
return active_pfn_set_overlap(pfn, --overlap); return active_cacheline_set_overlap(cln, --overlap);
} }
static int active_pfn_insert(struct dma_debug_entry *entry) static int active_cacheline_insert(struct dma_debug_entry *entry)
{ {
phys_addr_t cln = to_cacheline_number(entry);
unsigned long flags; unsigned long flags;
int rc; int rc;
/* If the device is not writing memory then we don't have any
* concerns about the cpu consuming stale data. This mitigates
* legitimate usages of overlapping mappings.
*/
if (entry->direction == DMA_TO_DEVICE)
return 0;
spin_lock_irqsave(&radix_lock, flags); spin_lock_irqsave(&radix_lock, flags);
rc = radix_tree_insert(&dma_active_pfn, entry->pfn, entry); rc = radix_tree_insert(&dma_active_cacheline, cln, entry);
if (rc == -EEXIST) if (rc == -EEXIST)
active_pfn_inc_overlap(entry->pfn); active_cacheline_inc_overlap(cln);
spin_unlock_irqrestore(&radix_lock, flags); spin_unlock_irqrestore(&radix_lock, flags);
return rc; return rc;
} }
static void active_pfn_remove(struct dma_debug_entry *entry) static void active_cacheline_remove(struct dma_debug_entry *entry)
{ {
phys_addr_t cln = to_cacheline_number(entry);
unsigned long flags; unsigned long flags;
/* ...mirror the insert case */
if (entry->direction == DMA_TO_DEVICE)
return;
spin_lock_irqsave(&radix_lock, flags); spin_lock_irqsave(&radix_lock, flags);
/* since we are counting overlaps the final put of the /* since we are counting overlaps the final put of the
* entry->pfn will occur when the overlap count is 0. * cacheline will occur when the overlap count is 0.
* active_pfn_dec_overlap() returns -1 in that case * active_cacheline_dec_overlap() returns -1 in that case
*/ */
if (active_pfn_dec_overlap(entry->pfn) < 0) if (active_cacheline_dec_overlap(cln) < 0)
radix_tree_delete(&dma_active_pfn, entry->pfn); radix_tree_delete(&dma_active_cacheline, cln);
spin_unlock_irqrestore(&radix_lock, flags); spin_unlock_irqrestore(&radix_lock, flags);
} }
/** /**
* debug_dma_assert_idle() - assert that a page is not undergoing dma * debug_dma_assert_idle() - assert that a page is not undergoing dma
* @page: page to lookup in the dma_active_pfn tree * @page: page to lookup in the dma_active_cacheline tree
* *
* Place a call to this routine in cases where the cpu touching the page * Place a call to this routine in cases where the cpu touching the page
* before the dma completes (page is dma_unmapped) will lead to data * before the dma completes (page is dma_unmapped) will lead to data
@ -536,22 +559,38 @@ static void active_pfn_remove(struct dma_debug_entry *entry)
*/ */
void debug_dma_assert_idle(struct page *page) void debug_dma_assert_idle(struct page *page)
{ {
static struct dma_debug_entry *ents[CACHELINES_PER_PAGE];
struct dma_debug_entry *entry = NULL;
void **results = (void **) &ents;
unsigned int nents, i;
unsigned long flags; unsigned long flags;
struct dma_debug_entry *entry; phys_addr_t cln;
if (!page) if (!page)
return; return;
cln = (phys_addr_t) page_to_pfn(page) << CACHELINE_PER_PAGE_SHIFT;
spin_lock_irqsave(&radix_lock, flags); spin_lock_irqsave(&radix_lock, flags);
entry = radix_tree_lookup(&dma_active_pfn, page_to_pfn(page)); nents = radix_tree_gang_lookup(&dma_active_cacheline, results, cln,
CACHELINES_PER_PAGE);
for (i = 0; i < nents; i++) {
phys_addr_t ent_cln = to_cacheline_number(ents[i]);
if (ent_cln == cln) {
entry = ents[i];
break;
} else if (ent_cln >= cln + CACHELINES_PER_PAGE)
break;
}
spin_unlock_irqrestore(&radix_lock, flags); spin_unlock_irqrestore(&radix_lock, flags);
if (!entry) if (!entry)
return; return;
cln = to_cacheline_number(entry);
err_printk(entry->dev, entry, err_printk(entry->dev, entry,
"DMA-API: cpu touching an active dma mapped page " "DMA-API: cpu touching an active dma mapped cacheline [cln=%pa]\n",
"[pfn=0x%lx]\n", entry->pfn); &cln);
} }
/* /*
@ -568,9 +607,9 @@ static void add_dma_entry(struct dma_debug_entry *entry)
hash_bucket_add(bucket, entry); hash_bucket_add(bucket, entry);
put_hash_bucket(bucket, &flags); put_hash_bucket(bucket, &flags);
rc = active_pfn_insert(entry); rc = active_cacheline_insert(entry);
if (rc == -ENOMEM) { if (rc == -ENOMEM) {
pr_err("DMA-API: pfn tracking ENOMEM, dma-debug disabled\n"); pr_err("DMA-API: cacheline tracking ENOMEM, dma-debug disabled\n");
global_disable = true; global_disable = true;
} }
@ -631,7 +670,7 @@ static void dma_entry_free(struct dma_debug_entry *entry)
{ {
unsigned long flags; unsigned long flags;
active_pfn_remove(entry); active_cacheline_remove(entry);
/* /*
* add to beginning of the list - this way the entries are * add to beginning of the list - this way the entries are