On 09/14/2016 11:38 PM, Oleg Nesterov wrote:
On 09/13, Dave Hansen wrote:

On 09/13/2016 07:59 AM, Oleg Nesterov wrote:
I agree. I don't even understand why this was considered as a bug.
Obviously, m_stop() which drops mmap_sep should not be called, or
all the threads should be stopped, if you want to trust the result.

There was a mapping at a given address.  That mapping did not change, it
was not split, its attributes did not change.  But, it didn't show up
when reading smaps.  Folks _actually_ noticed this in a test suite
looking for that address range in smaps.

I understand, and I won't argue with any change which makes the things
better. Just I do not think this is a real problem. And this patch can't
fix other oddities and it seems it adds another one (at least) although
I can easily misread this patch and/or the code.

So we change m_cache_vma(),

        -        m->version = m_next_vma(m->private, vma) ? vma->vm_start : 
-1UL;
        +        m->version = m_next_vma(m->private, vma) ? vma->vm_end : -1UL;

OK, and another change in m_start()

        -        if (vma && (vma = m_next_vma(priv, vma)))
        +        if (vma)

means that it can return the same vma if it grows in between.

show_map_vma() has another change

        +       start = max(vma->vm_start, start);

so it will be reported as _another_ vma, and this doesn't look exactly
right.

We noticed it in the discussion of v1, however it is not bad as Dave said
it is about 'address range' rather that vma.


And after that *ppos will be falsely incremented... but probably this
doesn't matter because the "if (pos < mm->map_count)" logic in m_start()
looks broken anyway.

The 'broken' can happen only if it is not the first read and m->version is
zero (*ppos != 0 && m->version == 0). If i understand the code correctly,
only m->buffer overflowed can trigger this, for smaps, each vma only
uses ~1k memory that means this could not happen. Right?




Reply via email to