lkml.org 
[lkml]   [2014]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFCv2 2/3] staging: ion: debugfs to shrink pool


2014-10-25 오전 5:38, Laura Abbott 쓴 글:
> Hi,
>
> On 10/23/2014 11:47 PM, Gioh Kim wrote:
>> This patch creates debugfs files, /sys/kernel/debug/ion/heaps/system_shrink,
>> to shrink pool or get pool size.
>> Reading the file returns pool size and writing occurs to shrink pool.
>>
>
> Can you clarify here that you are updating the existing debugfs
> infrastructure? Since the shrinker debugfs was commented out before
> it missed the API update so it would be good to note that as well.

OK, I will at the v2 patch.

>
>> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
>> ---
>> drivers/staging/android/ion/ion.c | 31 ++++++++-----------------------
>> 1 file changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index 290d4d2..ecc1121 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -1463,43 +1463,29 @@ static const struct file_operations debug_heap_fops = {
>> .release = single_release,
>> };
>>
>> -#ifdef DEBUG_HEAP_SHRINKER
>
> We're now unconditionally adding the shrinker debugfs. I'm not sure if
> this is something we want in production code. Could you turn this
> into a proper Kconfig?

The interface is via debugfs.
I think we don't turn on the debugfs in production code.

>
>> static int debug_shrink_set(void *data, u64 val)
>> {
>> struct ion_heap *heap = data;
>> - struct shrink_control sc;
>> int objs;
>>
>> - sc.gfp_mask = -1;
>> - sc.nr_to_scan = 0;
>> -
>> - if (!val)
>> - return 0;
>> -
>> - objs = heap->shrinker.shrink(&heap->shrinker, &sc);
>> - sc.nr_to_scan = objs;
>> + if (val)
>> + objs = val;
>> + else
>> + objs = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>>
>> - heap->shrinker.shrink(&heap->shrinker, &sc);
>> + (void)heap->ops->shrink(heap, __GFP_HIGHMEM, objs);
>
> The shrinker API now has separate functions for counting and scanning
> and ion wraps the calls to those as well to account for the deferred
> freelist. I realize the existing behavior may have been broken but the
> debugfs seems incomplete if it's not taking that into account as well.

ion_heap_shrink_count() and ion_heap_shrink_scan() are called by shrinker.
I think debug_shrink_set()/get() are for debugfs, so they can work without shrinker.

You can count deferred freelist via other debugfs file, for instance /sys/kernel/debug/ion/heaps/system.
I think debug_shrink_set()/get() should work by the number of pages in the pool.


>
>> return 0;
>> }
>>
>> static int debug_shrink_get(void *data, u64 *val)
>> {
>> struct ion_heap *heap = data;
>> - struct shrink_control sc;
>> - int objs;
>> -
>> - sc.gfp_mask = -1;
>> - sc.nr_to_scan = 0;
>> -
>> - objs = heap->shrinker.shrink(&heap->shrinker, &sc);
>> - *val = objs;
>> + *val = heap->ops->shrink(heap, __GFP_HIGHMEM, 0);
>> return 0;
>> }
>>
>> DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
>> debug_shrink_set, "%llu\n");
>> -#endif
>>
>> void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>> {
>> @@ -1534,8 +1520,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>> path, heap->name);
>> }
>>
>> -#ifdef DEBUG_HEAP_SHRINKER
>> - if (heap->shrinker.shrink) {
>> + if (heap->ops->shrink) {
>
> Technically a heap doesn't need to set ops->shrink to have a shrinker
> set up (not that it really matters for the current setup). Checking
> for scan_objects would be better.
>
>> char debug_name[64];
>>
>> snprintf(debug_name, 64, "%s_shrink", heap->name);
>> @@ -1550,7 +1535,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>> path, debug_name);
>> }
>> }
>> -#endif
>> +
>> up_write(&dev->lock);
>> }
>>
>>
>
> Thanks,
> Laura
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-10-27 02:01    [W:0.362 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site