Messages in this thread | | | Subject | Re: [PATCH 3/4] dma-debug: Dynamically expand the dma_debug_entry pool | From | John Garry <> | Date | Tue, 4 Dec 2018 16:30:47 +0000 |
| |
On 04/12/2018 13:11, Robin Murphy wrote: > Hi John, > > On 03/12/2018 18:23, John Garry wrote: >> On 03/12/2018 17:28, Robin Murphy wrote: >>> Certain drivers such as large multi-queue network adapters can use pools >>> of mapped DMA buffers larger than the default dma_debug_entry pool of >>> 65536 entries, with the result that merely probing such a device can >>> cause DMA debug to disable itself during boot unless explicitly given an >>> appropriate "dma_debug_entries=..." option. >>> >>> Developers trying to debug some other driver on such a system may not be >>> immediately aware of this, and at worst it can hide bugs if they fail to >>> realise that dma-debug has already disabled itself unexpectedly by the >>> time the code of interest gets to run. Even once they do realise, it can >>> be a bit of a pain to emprirically determine a suitable number of >>> preallocated entries to configure without massively over-allocating. >>> >>> There's really no need for such a static limit, though, since we can >>> quite easily expand the pool at runtime in those rare cases that the >>> preallocated entries are insufficient, which is arguably the least >>> surprising and most useful behaviour. >> >> Hi Robin, >> >> Do you have an idea on shrinking the pool again when the culprit >> driver is removed, i.e. we have so many unused debug entries now >> available? > > I honestly don't believe it's worth the complication. This is a > development feature with significant overheads already, so there's not > an awful lot to gain by trying to optimise memory usage. If a system can > ever load a driver that makes hundreds of thousands of simultaneous > mappings, it can almost certainly spare 20-odd megabytes of RAM for the > corresponding debug entries in perpetuity. Sure, it does mean you'd need > to reboot to recover memory from a major leak, but that's mostly true of > the current behaviour too, and rebooting during driver development is > hardly an unacceptable inconvenience. >
ok, I just thought that it would not be too difficult to implement this on the dma entry free path.
> In fact, having got this far in, what I'd quite like to do is to get rid > of dma_debug_resize_entries() such that we never need to free things at > all, since then we could allocate whole pages as blocks of entries to > save on masses of individual slab allocations. >
On a related topic, is it possible for the user to learn the total entries created at a given point in time? If not, could we add a file in the debugfs folder for this?
Thanks, John
> Robin. > >> >> Thanks, >> John >> >>> >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>> --- >>> kernel/dma/debug.c | 18 +++++++++++++++--- >>> 1 file changed, 15 insertions(+), 3 deletions(-) >>> >>> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c >>> index de5db800dbfc..46cc075aec99 100644 >>> --- a/kernel/dma/debug.c >>> +++ b/kernel/dma/debug.c >>> @@ -47,6 +47,9 @@ >>> #ifndef PREALLOC_DMA_DEBUG_ENTRIES >>> #define PREALLOC_DMA_DEBUG_ENTRIES (1 << 16) >>> #endif >>> +/* If the pool runs out, try this many times to allocate this many >>> new entries */ >>> +#define DMA_DEBUG_DYNAMIC_ENTRIES 256 >>> +#define DMA_DEBUG_DYNAMIC_RETRIES 2 >>> >>> enum { >>> dma_debug_single, >>> @@ -702,12 +705,21 @@ static struct dma_debug_entry >>> *dma_entry_alloc(void) >>> { >>> struct dma_debug_entry *entry; >>> unsigned long flags; >>> + int retry_count; >>> >>> - spin_lock_irqsave(&free_entries_lock, flags); >>> + for (retry_count = 0; ; retry_count++) { >>> + spin_lock_irqsave(&free_entries_lock, flags); >>> + >>> + if (num_free_entries > 0) >>> + break; >>> >>> - if (list_empty(&free_entries)) { >>> - global_disable = true; >>> spin_unlock_irqrestore(&free_entries_lock, flags); >>> + >>> + if (retry_count < DMA_DEBUG_DYNAMIC_RETRIES && >>> + !prealloc_memory(DMA_DEBUG_DYNAMIC_ENTRIES)) >>> + continue; >>> + >>> + global_disable = true; >>> pr_err("debugging out of memory - disabling\n"); >>> return NULL; >>> } >>> >> >> > > . >
| |