On 04/18/2018 08:49 PM, Matthew Wilcox wrote:
> From: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
>
> Use page->private instead, now that these two fields are in the same
> location. Include a compile-time assert that the fields don't get out
> of sync.
>
> Signed-off-by: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>
Why not retain a small union of "counters" and inuse/objects/frozens
within the SLUB's sub-structure? IMHO it would be more obvious and
reduce churn?
...
> @@ -358,17 +359,10 @@ static __always_inline void slab_unlock(struct page *page)
>
> static inline void set_page_slub_counters(struct page *page, unsigned long counters_new)
> {
> - struct page tmp;
> - tmp.counters = counters_new;
> - /*
> - * page->counters can cover frozen/inuse/objects as well
> - * as page->_refcount. If we assign to ->counters directly
> - * we run the risk of losing updates to page->_refcount, so
> - * be careful and only assign to the fields we need.
> - */
> - page->frozen = tmp.frozen;
> - page->inuse = tmp.inuse;
> - page->objects = tmp.objects;
BTW was this ever safe to begin with? IIRC bitfields are frowned upon as
a potential RMW. Or is there still at least guarantee the RMW happens
only within the 32bit struct and not the whole 64bit word, which used to
include also _refcount?
