Thread (38 messages) 38 messages, 5 authors, 2017-12-21

Re: [PATCH 2/8] mm: De-indent struct page

From: Michal Hocko <mhocko@kernel.org>
Date: 2017-12-18 20:49:38

On Mon 18-12-17 08:19:02, Matthew Wilcox wrote:
quoted hunk ↗ jump to hunk
On Mon, Dec 18, 2017 at 04:36:52PM +0100, Michal Hocko wrote:
quoted
On Sat 16-12-17 08:44:19, Matthew Wilcox wrote:
quoted
From: Matthew Wilcox <redacted>

I found the struct { union { struct { union { struct { } } } } }
layout rather confusing.  Fortunately, there is an easier way to write
this.  The innermost union is of four things which are the size of an
int, so the ones which are used by slab/slob/slub can be pulled up
two levels to be in the outermost union with 'counters'.  That leaves
us with struct { union { struct { atomic_t; atomic_t; } } } which
has the same layout, but is easier to read.
This is where the pahole output would be really helpeful. The patch
looks OK, I will double check with a fresh brain tomorrow (with the rest
of the series), though.
I got Arnaldo to change the pahole output to make this easier.  Here's
the result:
@@ -11,17 +11,15 @@
 	};                                               /*    16     8 */
 	union {
 		long unsigned int  counters;             /*    24     8 */
+		unsigned int       active;               /*    24     4 */
 		struct {
-			union {
-				atomic_t _mapcount;      /*    24     4 */
-				unsigned int active;     /*    24     4 */
-				struct {
-					unsigned int inuse:16; /*    24:16  4 */
-					unsigned int objects:15; /*    24: 1  4 */
-					unsigned int frozen:1; /*    24: 0  4 */
-				};                       /*    24     4 */
-				int units;               /*    24     4 */
-			};                               /*    24     4 */
+			unsigned int inuse:16;           /*    24:16  4 */
+			unsigned int objects:15;         /*    24: 1  4 */
+			unsigned int frozen:1;           /*    24: 0  4 */
+		};                                       /*    24     4 */
+		int                units;                /*    24     4 */
+		struct {
+			atomic_t   _mapcount;            /*    24     4 */
 			atomic_t   _refcount;            /*    28     4 */
 		};                                       /*    24     8 */
 	};                                               /*    24     8 */

It's even more dramatic if you use diff -uw (ignore whitespace):
@@ -11,9 +11,6 @@
 	};                                               /*    16     8 */
 	union {
 		long unsigned int  counters;             /*    24     8 */
-		struct {
-			union {
-				atomic_t _mapcount;      /*    24     4 */
 				unsigned int active;     /*    24     4 */
 				struct {
 					unsigned int inuse:16; /*    24:16  4 */
@@ -21,7 +18,8 @@
 					unsigned int frozen:1; /*    24: 0  4 */
 				};                       /*    24     4 */
 				int units;               /*    24     4 */
-			};                               /*    24     4 */
+		struct {
+			atomic_t   _mapcount;            /*    24     4 */
 			atomic_t   _refcount;            /*    28     4 */
 		};                                       /*    24     8 */
 	};                                               /*    24     8 */
Excelent! Could you add the later one to the changelog please? With
that
Acked-by: Michal Hocko <mhocko@suse.com>

I will go over the rest of the series tomorrow.

-- 
Michal Hocko
SUSE Labs

--
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help