On 09/10/2018 11:27 AM, Greg KH wrote:
> On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
>> The heap statistics have been removed and currently even basics statistics
>> are missing.
>>
>> This patch creates per heap debugfs directory /sys/kernel/debug/<heap_name>
>> and adds two counters - the number of allocated buffers and number of
>> allocated bytes.
>
> Why? What can we do with this information?
Are you in doubt that we need these two specific counters or debugfs at all?
>
>>
>> Signed-off-by: Alexey Skidanov <alexey.skidanov@xxxxxxxxx>
>> ---
>> drivers/staging/android/ion/ion.c | 40 +++++++++++++++++++++++++++++++--------
>> drivers/staging/android/ion/ion.h | 3 ++-
>> 2 files changed, 34 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index 9907332..62335b9 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -95,6 +95,9 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
>> goto err1;
>> }
>>
>> + heap->num_of_buffers++;
>> + heap->num_of_alloc_bytes += len;
>> +
>> INIT_LIST_HEAD(&buffer->attachments);
>> mutex_init(&buffer->lock);
>> mutex_lock(&dev->buffer_lock);
>> @@ -528,6 +531,8 @@ void ion_device_add_heap(struct ion_heap *heap)
>> {
>> struct ion_device *dev = internal_dev;
>> int ret;
>> + struct dentry *heap_root;
>> + char debug_name[64];
>>
>> if (!heap->ops->allocate || !heap->ops->free)
>> pr_err("%s: can not add heap with invalid ops struct.\n",
>> @@ -546,6 +551,33 @@ void ion_device_add_heap(struct ion_heap *heap)
>> }
>>
>> heap->dev = dev;
>> + heap->num_of_buffers = 0;
>> + heap->num_of_alloc_bytes = 0;
>> +
>> + /* Create heap root directory. If creation is failed, we may continue */
>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>> + if (!IS_ERR_OR_NULL(heap_root)) {
>
> Close, but no, never check the return value here. Just keep on moving,
> and call further debugfs calls. No need even for a comment here.
>
>> + debugfs_create_u64("num_of_buffers",
>> + 0444, heap_root,
>> + &heap->num_of_buffers);
>> + debugfs_create_u64("num_of_alloc_bytes",
>> + 0444,
>> + heap_root,
>> + &heap->num_of_alloc_bytes);
>> +
>> + if (heap->shrinker.count_objects &&
>> + heap->shrinker.scan_objects) {
>> + snprintf(debug_name, 64, "%s_shrink", heap->name);
>> + debugfs_create_file(debug_name,
>> + 0644,
>> + heap_root,
>> + heap,
>> + &debug_shrink_fops);
>> + }
>> + } else {
>> + pr_err("%s: Failed to create heap dir in debugfs\n", __func__);
>
> No need to print anything.
>
>> + }
>> +
>> down_write(&dev->lock);
>> heap->id = heap_id++;
>> /*
>> @@ -555,14 +587,6 @@ void ion_device_add_heap(struct ion_heap *heap)
>> plist_node_init(&heap->node, -heap->id);
>> plist_add(&heap->node, &dev->heaps);
>>
>> - if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
>> - char debug_name[64];
>> -
>> - snprintf(debug_name, 64, "%s_shrink", heap->name);
>> - debugfs_create_file(debug_name, 0644, dev->debug_root,
>> - heap, &debug_shrink_fops);
>> - }
>
> You just moved the location of these debugfs files, right? What
> userspace tool just broke when you did that? :)
>
> thanks,
>
> greg k-h
>
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
