Re: [PATCH v3 1/2] mm: migrate: prevent racy access to tlb_flush_pending
From: Sergey Senozhatsky <hidden>
Date: 2017-07-28 01:34:12
Also in:
linux-mm
just my 5 silly cents, On (07/27/17 04:40), Nadav Amit wrote: [..]
quoted hunk ↗ jump to hunk
static inline void set_tlb_flush_pending(struct mm_struct *mm) { - mm->tlb_flush_pending = true; + atomic_inc(&mm->tlb_flush_pending); /* * Guarantee that the tlb_flush_pending store does not leak into the@@ -544,7 +544,7 @@ static inline void set_tlb_flush_pending(struct mm_struct *mm) static inline void clear_tlb_flush_pending(struct mm_struct *mm) { barrier(); - mm->tlb_flush_pending = false; + atomic_dec(&mm->tlb_flush_pending); }
so, _technically_, set_tlb_flush_pending() can be nested, right? IOW, set_tlb_flush_pending() set_tlb_flush_pending() flush_tlb_range() clear_tlb_flush_pending() clear_tlb_flush_pending() // if we miss this one, then // ->tlb_flush_pending is !clear, // even though we called // clear_tlb_flush_pending() if so then set_ and clear_ are a bit misleading names for something that does atomic_inc()/atomic_dec() internally. especially when one sees this part
- clear_tlb_flush_pending(mm); +#if defined(CONFIG_NUMA_BALANCING) || defined(CONFIG_COMPACTION) + atomic_set(&mm->tlb_flush_pending, 0); +#endif
so we have clear_tlb_flush_pending() function which probably should set it to 0 as the name suggests (I see what you did tho), yet still do atomic_set() under ifdef-s. well, just nitpicks. -ss -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>