On 27.03.2024 14:28, Julien Grall wrote: > Hi Carlo, > > On 27/03/2024 11:10, Carlo Nonato wrote: >> Hi guys, >> >>> Question is: How would you justify such a change? IOW I'm not convinced >>> (yet) this wants doing there. >> >> You mean in this series? >> >>> Looking at the code, the flag is originally set in >>> alloc_domheap_pages(). So I guess it would make sense to do it in >>> free_domheap_pages(). >> >> We don't hold the heap_lock there. > Regardless of the safety question (I will answer below), count_info is > not meant to be protected by heap_lock. The lock is protecting the heap > and ensure we are not corrupting them when there are concurrent call to > free_heap_pages(). > > > Is it safe to change count_info without it? > > count_info is meant to be accessed locklessly. The flag PGC_extra cannot > be set/clear concurrently because you can't allocate a page that has not > yet been freed. > > So it would be fine to use clear_bit(..., ...);
Actually we hardly ever use clear_bit() on count_info. Normally we use ordinary C operators. Atomic (and otherwise lockless) updates are useful only if done like this everywhere. Jan
