On Thu, Sep 22, 2011 at 2:07 PM, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Thu, 2011-09-22 at 13:57 -0700, Colin Cross wrote:
>> seq_files are often used for debugging. When things are going wrong
>> due to failed physically contiguous allocations, the exponentially
>> growing physically contiguous allocations in seq_read can make things
>> worse. There is no need for physically contiguous memory, so switch
>> to virtually contiguous memory instead.
>
> vmalloc's are relatively expensive.
> Perhaps use kmalloc when appropriate instead?
Talking about allocation efficiency in the context of seq_files is
silly - for a KMALLOC_MAX_SIZE buffer (8MB), you are already going to
allocate 11 times, with increasingly large buffers, and are likely to
fail long before you ever get to KMALLOC_MAX_SIZE.
> []
>> - /* don't ask for more than the kmalloc() max size */
>> - if (size > KMALLOC_MAX_SIZE)
>> - size = KMALLOC_MAX_SIZE;
>> -
>> - buf = kmalloc(size, GFP_KERNEL);
>> + buf = vmalloc(size);
>> if (!buf)
>> return -ENOMEM;
>
> if (size > KMALLOC_MAX_SIZE)
> buf = vmalloc(size, GFP_KERNEL)
> else
> buf = kmalloc(size, GFP_KERNEL);
KMALLOC_MAX_SIZE is far too big for this. KMALLOC_MAX_SIZE is the
maximum allocation that is theoretically possible, but will fail if
you don't have any completely empty pageblocks. If I were to put a
size here, it would probably be order 3, but even that can easily fail
on a system that has under memory pressure.
>> + vfree(m->buf);
>
> if (m->size > KMALLOC_MAX_SIZE)
> vfree(m->buf);
> else
> kfree(m->buf);
>
>> m->buf = buf;
>> m->size = size;
>>
>> @@ -106,7 +103,7 @@ static int traverse(struct seq_file *m, loff_t offset)
>> return 0;
>> }
>> if (!m->buf) {
>> - m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);
>> + m->buf = vmalloc(m->size = PAGE_SIZE);
>
> embedding the set of m->size like this is ugly.
I agree, but it was there in the original file. I could clean it up,
but that should be in a separate patch.
> [do the same as above kmalloc/vmalloc based on size]
>
> etc.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
