Re: [PATCH v4 07/16] slub: Remove page->counters

[Matthew Wilcox <willy@infradead.org>]

On Tue, May 01, 2018 at 11:48:53AM -0500, Christopher Lameter wrote:
> On Mon, 30 Apr 2018, Matthew Wilcox wrote:
> 
> > 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.
> 
> Hrm. This makes the source code a bit less readable. Guess its ok.
> 
> Acked-by: Christoph Lameter <cl@linux.com>

Thanks for the ACK.  I'm not thrilled with this particular patch, but
I'm not thrilled with any of the other options we've come up with either.

Option 1:

Patch as written.
Pro: Keeps struct page simple
Con: Hidden dependency on page->private and page->inuse being in the same bits

Option 2:

@@ -113,9 +113,14 @@ struct page {
                        struct kmem_cache *slub_cache;  /* shared with slab */
                        /* Double-word boundary */
                        void *slub_freelist;            /* shared with slab */
-                       unsigned inuse:16;
-                       unsigned objects:15;
-                       unsigned frozen:1;
+                       union {
+                               unsigned long counters;
+                               struct {
+                                       unsigned inuse:16;
+                                       unsigned objects:15;
+                                       unsigned frozen:1;
+                               };
+                       };
                };
                struct {        /* Tail pages of compound page */
                        unsigned long compound_head;    /* Bit zero is set */

Pro: Expresses exactly what we do.
Con: Back to five levels of indentation in struct page

Option 3: Use -fms-extensions to create a slub_page structure.

Pro: Indentation reduced to minimum and no cross-union dependencies
Con: Nobody seemed interested in the idea

Option 4: Use explicit shifting-and-masking to combine the three counters
into one word.

Con: Lots of churn.