[RFC PATCH 1/2] Shrink compat_ioctl.c

Previous thread: [PATCH] oom-killer kills more than needed by Chad Zanonie on Monday, September 29, 2008 - 4:09 pm. (1 message)

Next thread: Please pull arch perfmon update by Andi Kleen on Monday, September 29, 2008 - 4:52 pm. (7 messages)
From: Matt Mackall
Date: Monday, September 29, 2008 - 4:27 pm

I'm throwing this out untested as I don't have a mixed 64/32 system
handy at the moment.

compat_ioctl: shrink structures

Initially, compat_ioctl.c has a ~18k table of translated ioctls.
Each table entry is 24 bytes but we can shrink this to 16:

- use short table indexes rather than a pointer for .next values
- use unsigned ints for cmd numbers (they're 32-bit ioctls, after all)

In addition, there's a 2k hash table that we can do away with simply
by hashifying the main table in place at init time.

This saves about 6k data and 2k BSS.

diff -r f6d1daaf90b2 -r b055e64aea17 fs/compat_ioctl.c
--- a/fs/compat_ioctl.c	Mon Sep 29 17:28:28 2008 -0500
+++ b/fs/compat_ioctl.c	Mon Sep 29 17:28:41 2008 -0500
@@ -1825,21 +1825,21 @@
 					unsigned long, struct file *);
 
 struct ioctl_trans {
-	unsigned long cmd;
 	ioctl_trans_handler_t handler;
-	struct ioctl_trans *next;
+	unsigned int cmd;
+	unsigned short next;
 };
 
 #define HANDLE_IOCTL(cmd,handler) \
-	{ (cmd), (ioctl_trans_handler_t)(handler) },
+	{ (ioctl_trans_handler_t)(handler), (cmd)},
 
 /* pointer to compatible structure or no argument */
 #define COMPATIBLE_IOCTL(cmd) \
-	{ (cmd), do_ioctl32_pointer },
+	{ do_ioctl32_pointer, (cmd) },
 
 /* argument is an unsigned long integer, not a pointer */
 #define ULONG_IOCTL(cmd) \
-	{ (cmd), (ioctl_trans_handler_t)sys_ioctl },
+	{ (ioctl_trans_handler_t)sys_ioctl, (cmd) },
 
 /* ioctl should not be warned about even if it's not implemented.
    Valid reasons to use this:
@@ -1850,7 +1850,7 @@
    Most other reasons are not valid. */
 #define IGNORE_IOCTL(cmd) COMPATIBLE_IOCTL(cmd)
 
-static struct ioctl_trans ioctl_start[] = {
+static struct ioctl_trans ioctl_table[] = {
 /* compatible ioctls first */
 COMPATIBLE_IOCTL(0x4B50)   /* KDGHWCLK - not in the kernel, but don't complain */
 COMPATIBLE_IOCTL(0x4B51)   /* KDSHWCLK - not in the kernel, but don't complain */
@@ -2728,10 +2728,9 @@
 #endif
 };
 
-#define IOCTL_HASHSIZE 256
-static struct ...
From: Matt Mackall
Date: Monday, September 29, 2008 - 4:29 pm

compat-ioctl: further compactify table

Most entries use the basic compat handler, which means for most
entries it's redundant. Rather than store a handler for each entry, we
split the table so that there are two entry types, the 'basic stuff'
and the handler pointer.

At build time, the table is sorted so that each entry with a handler
pointer is two entries, handler pointer first. At init time we re-sort
the list so that all the handler pointers are at the end of the table
and each basic entry tracks the index that the corresponding handler
pointer is at.

This cuts the size of most entries in half, so the table goes from
~12k to ~7k.

diff -r b055e64aea17 -r 6841f3ce32be fs/compat_ioctl.c
--- a/fs/compat_ioctl.c	Mon Sep 29 17:28:41 2008 -0500
+++ b/fs/compat_ioctl.c	Mon Sep 29 17:28:44 2008 -0500
@@ -1825,21 +1825,26 @@
 					unsigned long, struct file *);
 
 struct ioctl_trans {
-	ioctl_trans_handler_t handler;
-	unsigned int cmd;
-	unsigned short next;
+	union {
+		struct {
+			unsigned int cmd;
+			short handle; /* index of entry containing fn ptr */
+			short next; /* index of next entry in hash chain */
+		} e;
+		ioctl_trans_handler_t handler;
+	} u;
 };
 
-#define HANDLE_IOCTL(cmd,handler) \
-	{ (ioctl_trans_handler_t)(handler), (cmd)},
+#define HANDLE_IOCTL(command, fn) \
+	{{ .handler = (ioctl_trans_handler_t)(fn) }},	\
+	{{ .e = { .cmd = (command), .handle = 1 }}},
+
+/* argument is an unsigned long integer, not a pointer */
+#define ULONG_IOCTL(cmd) HANDLE_IOCTL(cmd, sys_ioctl)
 
 /* pointer to compatible structure or no argument */
-#define COMPATIBLE_IOCTL(cmd) \
-	{ do_ioctl32_pointer, (cmd) },
-
-/* argument is an unsigned long integer, not a pointer */
-#define ULONG_IOCTL(cmd) \
-	{ (ioctl_trans_handler_t)sys_ioctl, (cmd) },
+#define COMPATIBLE_IOCTL(command) \
+	{{ .e = { .cmd = (command) }}},
 
 /* ioctl should not be warned about even if it's not implemented.
    Valid reasons to use this:
@@ -2814,8 +2819,8 @@
 		break;
 	}
 ...
From: Andi Kleen
Date: Monday, September 29, 2008 - 4:38 pm

You mean by using a closed hash?

Acked-by: Andi Kleen <ak@linux.intel.com>

-Andi
--

From: Matt Mackall
Date: Monday, September 29, 2008 - 4:38 pm

The original hash table was 256 pointers into the main table. I simply
rearrange the main table so the first 256 entries have an appropriate
hash. Basically:

for i in len(table):
  h = hash(table[i])
  swap(table[i], table[hash])

At the end of this loop, table[0:256] will contain an appropriate table
entry, if it exists. So no secondary table is needed.

-- 
Mathematics is the supreme nostalgia of our time.

--

Previous thread: [PATCH] oom-killer kills more than needed by Chad Zanonie on Monday, September 29, 2008 - 4:09 pm. (1 message)

Next thread: Please pull arch perfmon update by Andi Kleen on Monday, September 29, 2008 - 4:52 pm. (7 messages)