[patch] 2.5.20 x86 iobitmap cleanup

[patch] 2.5.20 x86 iobitmap cleanup

[Benjamin LaHaise]

Hey Linus et al,

Below is a patch against base 2.5.20 which makes the io port 
bitmap in thread_struct a pointer to a bitmap that is only 
allocated when the process calls ioperm.  This results in 132 
bytes being removed from struct task_struct.

		-ben
-- 
"You will be reincarnated as a toad; and you will be much happier."


diff -urN v2.5.20/arch/i386/kernel/ioport.c iobitmap-2.5.20.diff/arch/i386/kernel/ioport.c
--- v2.5.20/arch/i386/kernel/ioport.c	Mon Jul 19 18:22:48 1999
+++ iobitmap-2.5.20.diff/arch/i386/kernel/ioport.c	Thu Jun  6 00:54:22 2002
@@ -66,12 +66,15 @@
 	 * IO bitmap up. ioperm() is much less timing critical than clone(),
 	 * this is why we delay this operation until now:
 	 */
-	if (!t->ioperm) {
+	if (!t->ts_io_bitmap) {
+		unsigned long *bitmap = kmalloc((IO_BITMAP_SIZE+1)*4, GFP_KERNEL);
+		if (!bitmap)
+			return -ENOMEM;
 		/*
 		 * just in case ...
 		 */
-		memset(t->io_bitmap,0xff,(IO_BITMAP_SIZE+1)*4);
-		t->ioperm = 1;
+		memset(bitmap, 0xff, (IO_BITMAP_SIZE+1)*4);
+		t->ts_io_bitmap = bitmap;
 		/*
 		 * this activates it in the TSS
 		 */
@@ -81,7 +84,7 @@
 	/*
 	 * do it in the per-thread copy and in the TSS ...
 	 */
-	set_bitmap(t->io_bitmap, from, num, !turn_on);
+	set_bitmap(t->ts_io_bitmap, from, num, !turn_on);
 	set_bitmap(tss->io_bitmap, from, num, !turn_on);
 
 	return 0;
diff -urN v2.5.20/arch/i386/kernel/process.c iobitmap-2.5.20.diff/arch/i386/kernel/process.c
--- v2.5.20/arch/i386/kernel/process.c	Thu Jun  6 00:35:52 2002
+++ iobitmap-2.5.20.diff/arch/i386/kernel/process.c	Thu Jun  6 00:51:03 2002
@@ -685,8 +685,8 @@
 		loaddebug(next, 7);
 	}
 
-	if (unlikely(prev->ioperm || next->ioperm)) {
-		if (next->ioperm) {
+	if (unlikely(prev->ts_io_bitmap || next->ts_io_bitmap)) {
+		if (next->ts_io_bitmap) {
 			/*
 			 * 4 cachelines copy ... not good, but not that
 			 * bad either. Anyone got something better?
@@ -695,7 +695,7 @@
 			 * and playing VM tricks to switch the IO bitmap
 			 * is not really acceptable.]
 			 */
-			memcpy(tss->io_bitmap, next->io_bitmap,
+			memcpy(tss->io_bitmap, next->ts_io_bitmap,
 				 IO_BITMAP_SIZE*sizeof(unsigned long));
 			tss->bitmap = IO_BITMAP_OFFSET;
 		} else
diff -urN v2.5.20/include/asm-alpha/thread_info.h iobitmap-2.5.20.diff/include/asm-alpha/thread_info.h
--- v2.5.20/include/asm-alpha/thread_info.h	Thu Jun  6 00:35:33 2002
+++ iobitmap-2.5.20.diff/include/asm-alpha/thread_info.h	Thu Jun  6 01:01:05 2002
@@ -51,6 +51,7 @@
 #define free_thread_info(ti) free_pages((unsigned long) (ti), 1)
 #define get_thread_info(ti) get_task_struct((ti)->task)
 #define put_thread_info(ti) put_task_struct((ti)->task)
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 #endif /* __ASSEMBLY__ */
 
diff -urN v2.5.20/include/asm-arm/thread_info.h iobitmap-2.5.20.diff/include/asm-arm/thread_info.h
--- v2.5.20/include/asm-arm/thread_info.h	Thu Jun  6 00:36:03 2002
+++ iobitmap-2.5.20.diff/include/asm-arm/thread_info.h	Thu Jun  6 01:01:13 2002
@@ -82,6 +82,7 @@
 
 extern struct thread_info *alloc_thread_info(void);
 extern void free_thread_info(struct thread_info *);
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 #define get_thread_info(ti)	get_task_struct((ti)->task)
 #define put_thread_info(ti)	put_task_struct((ti)->task)
diff -urN v2.5.20/include/asm-i386/processor.h iobitmap-2.5.20.diff/include/asm-i386/processor.h
--- v2.5.20/include/asm-i386/processor.h	Thu Jun  6 00:35:53 2002
+++ iobitmap-2.5.20.diff/include/asm-i386/processor.h	Thu Jun  6 00:50:06 2002
@@ -369,8 +369,7 @@
 	unsigned long		screen_bitmap;
 	unsigned long		v86flags, v86mask, v86mode, saved_esp0;
 /* IO permissions */
-	int		ioperm;
-	unsigned long	io_bitmap[IO_BITMAP_SIZE+1];
+	unsigned long	*ts_io_bitmap;
 };
 
 #define INIT_THREAD  {						\
@@ -380,7 +379,7 @@
 	0, 0, 0,						\
 	{ { 0, }, },		/* 387 state */			\
 	0,0,0,0,0,0,						\
-	0,{~0,}			/* io permissions */		\
+	0,NULL,			/* io permissions */		\
 }
 
 #define INIT_TSS  {						\
diff -urN v2.5.20/include/asm-i386/thread_info.h iobitmap-2.5.20.diff/include/asm-i386/thread_info.h
--- v2.5.20/include/asm-i386/thread_info.h	Thu Jun  6 00:35:32 2002
+++ iobitmap-2.5.20.diff/include/asm-i386/thread_info.h	Thu Jun  6 01:00:13 2002
@@ -79,6 +79,7 @@
 #define free_thread_info(ti) free_pages((unsigned long) (ti), 1)
 #define get_thread_info(ti) get_task_struct((ti)->task)
 #define put_thread_info(ti) put_task_struct((ti)->task)
+#define arch_dispose_thread_struct(tsk)	kfree((tsk)->thread.ts_io_bitmap)
 
 #else /* !__ASSEMBLY__ */
 
diff -urN v2.5.20/include/asm-ia64/thread_info.h iobitmap-2.5.20.diff/include/asm-ia64/thread_info.h
--- v2.5.20/include/asm-ia64/thread_info.h	Thu Jun  6 00:35:33 2002
+++ iobitmap-2.5.20.diff/include/asm-ia64/thread_info.h	Thu Jun  6 01:01:39 2002
@@ -46,6 +46,7 @@
 
 /* how to get the thread information struct from C */
 #define current_thread_info() ((struct thread_info *) ((char *) current + IA64_TASK_SIZE))
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 #endif /* !__ASSEMBLY */
 
diff -urN v2.5.20/include/asm-ppc/thread_info.h iobitmap-2.5.20.diff/include/asm-ppc/thread_info.h
--- v2.5.20/include/asm-ppc/thread_info.h	Thu Jun  6 00:35:49 2002
+++ iobitmap-2.5.20.diff/include/asm-ppc/thread_info.h	Thu Jun  6 01:01:46 2002
@@ -55,6 +55,7 @@
 #define free_thread_info(ti)	free_pages((unsigned long) (ti), 1)
 #define get_thread_info(ti)	get_task_struct((ti)->task)
 #define put_thread_info(ti)	put_task_struct((ti)->task)
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 #endif /* __ASSEMBLY__ */
 
 /*
diff -urN v2.5.20/include/asm-ppc64/thread_info.h iobitmap-2.5.20.diff/include/asm-ppc64/thread_info.h
--- v2.5.20/include/asm-ppc64/thread_info.h	Thu Jun  6 00:36:03 2002
+++ iobitmap-2.5.20.diff/include/asm-ppc64/thread_info.h	Thu Jun  6 01:01:51 2002
@@ -50,6 +50,7 @@
 #define free_thread_info(ti)	free_pages((unsigned long) (ti), THREAD_ORDER)
 #define get_thread_info(ti)	get_task_struct((ti)->task)
 #define put_thread_info(ti)	put_task_struct((ti)->task)
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 #if THREAD_SIZE != (4*PAGE_SIZE)
 #error update vmlinux.lds and current_thread_info to match
diff -urN v2.5.20/include/asm-sparc64/thread_info.h iobitmap-2.5.20.diff/include/asm-sparc64/thread_info.h
--- v2.5.20/include/asm-sparc64/thread_info.h	Thu Jun  6 00:35:49 2002
+++ iobitmap-2.5.20.diff/include/asm-sparc64/thread_info.h	Thu Jun  6 01:02:03 2002
@@ -130,6 +130,7 @@
 /* how to get the thread information struct from C */
 register struct thread_info *current_thread_info_reg asm("g6");
 #define current_thread_info()	(current_thread_info_reg)
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 /* thread information allocation */
 #if PAGE_SHIFT == 13
diff -urN v2.5.20/include/asm-x86_64/thread_info.h iobitmap-2.5.20.diff/include/asm-x86_64/thread_info.h
--- v2.5.20/include/asm-x86_64/thread_info.h	Thu Jun  6 00:35:33 2002
+++ iobitmap-2.5.20.diff/include/asm-x86_64/thread_info.h	Thu Jun  6 01:02:08 2002
@@ -71,6 +71,7 @@
 #define free_thread_info(ti) free_pages((unsigned long) (ti), 1)
 #define get_thread_info(ti) get_task_struct((ti)->task)
 #define put_thread_info(ti) put_task_struct((ti)->task)
+#define arch_dispose_thread_struct(tsk)		do{}while(0)
 
 #else /* !__ASSEMBLY__ */
 
diff -urN v2.5.20/kernel/fork.c iobitmap-2.5.20.diff/kernel/fork.c
--- v2.5.20/kernel/fork.c	Thu Jun  6 00:35:50 2002
+++ iobitmap-2.5.20.diff/kernel/fork.c	Thu Jun  6 01:00:10 2002
@@ -123,6 +123,7 @@
 
 void __put_task_struct(struct task_struct *tsk)
 {
+	arch_dispose_thread_struct(tsk);
 	free_thread_info(tsk->thread_info);
 	kmem_cache_free(task_struct_cachep,tsk);
 }

Re: [patch] 2.5.20 x86 iobitmap cleanup

[Linus Torvalds]

On Thu, 6 Jun 2002, Benjamin LaHaise wrote:
> 
> Below is a patch against base 2.5.20 which makes the io port 
> bitmap in thread_struct a pointer to a bitmap that is only 
> allocated when the process calls ioperm.  This results in 132 
> bytes being removed from struct task_struct.

Why do you introduce a arch_dispose_thread_struct(), instead of just 
making the x86 exit_thread() do it?

		Linus

Re: [patch] 2.5.20 x86 iobitmap cleanup

[Benjamin LaHaise]

On Wed, Jun 12, 2002 at 01:04:09PM -0700, Linus Torvalds wrote:
> Why do you introduce a arch_dispose_thread_struct(), instead of just 
> making the x86 exit_thread() do it?

That makes much more sense.  Below is an updated patch against cset 
1.479 that allocs in sys_ioperm() and copy_thread() and frees in 
exit_thread().  It also gets rid of the IO_BITMAP_SIZE+1 crap, as 
only the tss actually needs the tail long, and we weren't copying 
it into the bitmap anyways.  There is a test program at 
http://www.kvack.org/~blah/iobitmap.c .   Side note: double kfree() 
does not trigger a BUG()...

		-ben
-- 
"You will be reincarnated as a toad; and you will be much happier."

:r ~/patches/v2.5/v2.5.21.5-io_bitmap-B0.diff
diff -urN linus-2.5/arch/i386/kernel/ioport.c work-2.5.diff/arch/i386/kernel/ioport.c
--- linus-2.5/arch/i386/kernel/ioport.c	Wed Jun 12 20:59:11 2002
+++ work-2.5.diff/arch/i386/kernel/ioport.c	Thu Jun 13 02:10:59 2002
@@ -10,10 +10,10 @@
 #include <linux/errno.h>
 #include <linux/types.h>
 #include <linux/ioport.h>
-#include <linux/mm.h>
 #include <linux/smp.h>
 #include <linux/smp_lock.h>
 #include <linux/stddef.h>
+#include <linux/slab.h>
 
 /* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
 static void set_bitmap(unsigned long *bitmap, short base, short extent, int new_value)
@@ -66,12 +66,16 @@
 	 * IO bitmap up. ioperm() is much less timing critical than clone(),
 	 * this is why we delay this operation until now:
 	 */
-	if (!t->ioperm) {
+	if (!t->ts_io_bitmap) {
+		unsigned long *bitmap;
+		bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
+		if (!bitmap)
+			return -ENOMEM;
 		/*
 		 * just in case ...
 		 */
-		memset(t->io_bitmap,0xff,(IO_BITMAP_SIZE+1)*4);
-		t->ioperm = 1;
+		memset(bitmap, 0xff, IO_BITMAP_BYTES);
+		t->ts_io_bitmap = bitmap;
 		/*
 		 * this activates it in the TSS
 		 */
@@ -81,7 +85,7 @@
 	/*
 	 * do it in the per-thread copy and in the TSS ...
 	 */
-	set_bitmap(t->io_bitmap, from, num, !turn_on);
+	set_bitmap(t->ts_io_bitmap, from, num, !turn_on);
 	set_bitmap(tss->io_bitmap, from, num, !turn_on);
 
 	return 0;
diff -urN linus-2.5/arch/i386/kernel/process.c work-2.5.diff/arch/i386/kernel/process.c
--- linus-2.5/arch/i386/kernel/process.c	Wed Jun 12 20:59:11 2002
+++ work-2.5.diff/arch/i386/kernel/process.c	Thu Jun 13 02:03:36 2002
@@ -509,7 +509,13 @@
  */
 void exit_thread(void)
 {
-	/* nothing to do ... */
+	struct task_struct *tsk = current;
+
+	/* The process may have allocated an io port bitmap... nuke it. */
+	if (unlikely(NULL != tsk->thread.ts_io_bitmap)) {
+		kfree(tsk->thread.ts_io_bitmap);
+		tsk->thread.ts_io_bitmap = NULL;
+	}
 }
 
 void flush_thread(void)
@@ -549,6 +555,7 @@
 	struct task_struct * p, struct pt_regs * regs)
 {
 	struct pt_regs * childregs;
+	struct task_struct *tsk;
 
 	childregs = ((struct pt_regs *) (THREAD_SIZE + (unsigned long) p->thread_info)) - 1;
 	struct_cpy(childregs, regs);
@@ -563,8 +570,17 @@
 	savesegment(fs,p->thread.fs);
 	savesegment(gs,p->thread.gs);
 
-	unlazy_fpu(current);
-	struct_cpy(&p->thread.i387, &current->thread.i387);
+	tsk = current;
+	unlazy_fpu(tsk);
+	struct_cpy(&p->thread.i387, &tsk->thread.i387);
+
+	if (unlikely(NULL != tsk->thread.ts_io_bitmap)) {
+		p->thread.ts_io_bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
+		if (!p->thread.ts_io_bitmap)
+			return -ENOMEM;
+		memcpy(p->thread.ts_io_bitmap, tsk->thread.ts_io_bitmap,
+			IO_BITMAP_BYTES);
+	}
 
 	return 0;
 }
@@ -685,8 +701,8 @@
 		loaddebug(next, 7);
 	}
 
-	if (unlikely(prev->ioperm || next->ioperm)) {
-		if (next->ioperm) {
+	if (unlikely(prev->ts_io_bitmap || next->ts_io_bitmap)) {
+		if (next->ts_io_bitmap) {
 			/*
 			 * 4 cachelines copy ... not good, but not that
 			 * bad either. Anyone got something better?
@@ -695,8 +711,8 @@
 			 * and playing VM tricks to switch the IO bitmap
 			 * is not really acceptable.]
 			 */
-			memcpy(tss->io_bitmap, next->io_bitmap,
-				 IO_BITMAP_SIZE*sizeof(unsigned long));
+			memcpy(tss->io_bitmap, next->ts_io_bitmap,
+				IO_BITMAP_BYTES);
 			tss->bitmap = IO_BITMAP_OFFSET;
 		} else
 			/*
diff -urN linus-2.5/include/asm-i386/processor.h work-2.5.diff/include/asm-i386/processor.h
--- linus-2.5/include/asm-i386/processor.h	Wed Jun 12 20:59:17 2002
+++ work-2.5.diff/include/asm-i386/processor.h	Thu Jun 13 02:02:30 2002
@@ -267,6 +267,7 @@
  * Size of io_bitmap in longwords: 32 is ports 0-0x3ff.
  */
 #define IO_BITMAP_SIZE	32
+#define IO_BITMAP_BYTES	(IO_BITMAP_SIZE * 4)
 #define IO_BITMAP_OFFSET offsetof(struct tss_struct,io_bitmap)
 #define INVALID_IO_BITMAP_OFFSET 0x8000
 
@@ -370,8 +371,7 @@
 	unsigned long		screen_bitmap;
 	unsigned long		v86flags, v86mask, v86mode, saved_esp0;
 /* IO permissions */
-	int		ioperm;
-	unsigned long	io_bitmap[IO_BITMAP_SIZE+1];
+	unsigned long	*ts_io_bitmap;
 };
 
 #define INIT_THREAD  {						\
@@ -381,7 +381,7 @@
 	0, 0, 0,						\
 	{ { 0, }, },		/* 387 state */			\
 	0,0,0,0,0,0,						\
-	0,{~0,}			/* io permissions */		\
+	NULL,			/* io permissions */		\
 }
 
 #define INIT_TSS  {						\