Re: [PATCH 10/10] mm,sched: conditionally skip lazy TLB mm refcounting

[Andy Lutomirski]

> On Jul 29, 2018, at 5:11 AM, Rik van Riel <riel@xxxxxxxxxxx> wrote:
> 
>> On Sat, 2018-07-28 at 21:21 -0700, Andy Lutomirski wrote:
>> On Sat, Jul 28, 2018 at 2:53 PM, Rik van Riel <riel@xxxxxxxxxxx>
>> wrote:
>>> Conditionally skip lazy TLB mm refcounting. When an architecture
>>> has
>>> CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is used in
>>> lazy TLB mode anywhere will get shot down from exit_mmap, and there
>>> in no need to incur the cache line bouncing overhead of refcounting
>>> a lazy TLB mm.
>> 
>> Unless I've misunderstood something, this patch results in idle tasks
>> whose active_mm has been freed still having active_mm pointing at
>> freed memory. 
> 
> Patch 9/10 is supposed to ensure that the lazy TLB CPUs get
> switched to init_mm before an mm is freed. No CPU should ever
> have its active_mm pointing at a freed mm.
> 
> Your message made me re-read the code, and now I realize that
> leave_mm does not actually do that.
> 
> Looking at the other callers of leave_mm, I might not be the
> only one surprised by that; xen_drop_mm_ref comes to mind.
> 
> I guess I should some code to leave_mm to have it actually
> clear active_mm and call the conditional refcount drop helper
> function.
> 
> Does that clear up the confusion?
> 

Kind of. But whatâs the point of keeping active_mm?  On architectures that opt in to the new mode, there wonât be any code that cares about itâs value.  Whatâs the benefit of keeping it around?  If you ifdef it out, then it canât possibly point to freed memory, and thereâs nothing to worry about.