[lttng-dev] [PATCH lttng-modules] Fix: Building the event list fails on fragmented memory

[Jan Glauber]

On Thu, Jun 25, 2015 at 01:37:29PM +0000, Mathieu Desnoyers wrote:
> ----- On Jun 25, 2015, at 9:00 AM, Mathieu Desnoyers mathieu.desnoyers at efficios.com wrote:
> 
> > ----- On Jun 25, 2015, at 4:08 AM, Jan Glauber jan.glauber at gmail.com wrote:
> > 
> >> On Tue, Jun 23, 2015 at 09:04:18PM +0000, Mathieu Desnoyers wrote:
> >>> Nope, it wasn't it. metadata_written is always <= cache_alloc, so the
> >>> current upstream code looks correct.
> >>> 
> >>> I've just tried after changing the CPU configuration from "hypervisor
> >>> default" to "copy host cpu config" on my machine, and was then unable
> >>> to reproduce. I wonder if there is not an issue with __memcpy implementation
> >>> for specific x86_64 processors (e.g. reading too many bytes from the vmalloc'd
> >>> area).
> >>> 
> >>> Thoughts ?
> >> 
> >> Just speculation without further debugging data from the hypervisor setup
> >> where it fails...
> >> 
> >> From my understanding memcpy should work regardless of kmalloc or
> >> vmalloc.
> >> 
> >> Maybe the length parameter of the memcpy is wrong?
> >> With kmalloc and the 1:1 mapping this wont matter but if the memcpy
> >> crosses a page it might fault if the next page is not vmalloc'ed.
> >> 
> >> But the fault address ffffc900038d995e looks more like src is completely
> >> messed up.
> >> 
> >> What is the difference between the CPU configurations you tried?
> > 
> > Actually, I just narrowed it down to a missing mutex around the metadata
> > cache accesses. The updater can reallocate the memory, and the reader
> > (which outputs from the cache to a ring buffer) assumes that it does not
> > have to lock the metadata cache because it only reads its content.
> > Unfortunately, this does not take into account that the memory backing
> > the cache can be reallocated.
> > 
> > I'll prepare a fix.

Nice catch!

--Jan