just my 5 silly cents,
On (07/27/17 04:40), Nadav Amit wrote:
[..]
> 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