On Mon, 6 Sep 2021 17:00:18 +0200
Vlastimil Babka <vbabka@xxxxxxx> wrote:
> On 9/3/21 04:21, Liam Howlett wrote:
> > Print to the trace log before releasing the lock to avoid racing with
> > other trace log printers of the same lock type.
>
> That description could use more details maybe?
Agreed, perhaps add something like this:
Due to the tracing of taking the mmap_lock happened outside the lock
itself, the trace can become confusing, making it look like more than one
task has the same lock:
task-749 [006] .... 14437980: mmap_lock_acquire_returned: mm=00000000c94d28b8 memcg_path= write=true success=true
task-750 [007] .... 14437981: mmap_lock_acquire_returned: mm=00000000c94d28b8 memcg_path= write=true success=true
task-749 [006] .... 14437983: mmap_lock_released: mm=00000000c94d28b8 memcg_path= write=true
When in actuality the following occurred:
task-749 [006] - take mmap_lock
task-749 [006] - trace taking of mmap_lock
task-749 [006] - release mmap_lock
task-750 [007] - take mmap_lock
task-750 [007] - trace taking of mmap_lock
task-749 [006] - trace releasing of mmap_lock
Although the mmap_lock was taken and released then taken again by another
process, the lack of protection around the tracing of the activity caused
it to show the events out of order. If the tracing of the taking and
releasing of the mmap_lock is done inside the lock, it will protect this
from happening.
Andrew, I see you took this into your tree. Not sure if you sent it to
Linus yet. Perhaps add the above to the patch if you have not yet sent it
(with Liam's permission of course).
Seeing that the patch has this as a link in the commit, at the very least,
the above explanation will be archived.
-- Steve
>
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> > Suggested-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
