lkml.org 
[lkml]   [2018]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/4] dma-debug: Dynamically expand the dma_debug_entry pool
From
Date
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;
>>> }
>>>
>>
>>
>
> .
>


\
 
 \ /
  Last update: 2018-12-04 17:32    [W:0.904 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site