Skip to contentSkip to conversation

smaller kernels

PATCH In review Andrew Morton 5 participants 9 messages Mar 20, 2002 – Mar 26, 2002

Subsystems (7)
Networking VFS NFS client MM core +3 more subsystems
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 smaller kernels

This is the result of a search-and-destroy mission against oft-repeated strings in the kernel text. These come about due to the toolchain's inability to common up strings between compilation units.

In particular, if there is a printk inside an inline function then the string gets expanded into every object file which included that inline function.

So the patch goes through and removes the offending strings in various ways.

Kernel size is reduced by another 110 kbytes in my build.

I haven't sent this to Linus. gcc-3.0.x does the right thing, and as 3.x will likely be the preferred compiler when 2.6 is released, this change isn't needed for Linus kernels. But it's needed for 2.4.

include/asm-i386/mmu_context.h renamed from include/asm-i386/mmu_context.h~inline-BUG +1 -1
@@ -48,7 +48,7 @@ static inline void switch_mm(struct mm_s
 	else {
 		cpu_tlbstate[cpu].state = TLBSTATE_OK;
 		if(cpu_tlbstate[cpu].active_mm != next)
-			BUG();
+			out_of_line_bug();
 		if(!test_and_set_bit(cpu, &next->cpu_vm_mask)) {
 			/* We were in lazy tlb mode and leave_mm disabled 
 			 * tlb flush IPI delivery. We must flush our tlb.
include/linux/kernel.h renamed from include/linux/kernel.h~inline-BUG +2 -0
@@ -181,4 +181,6 @@ struct sysinfo {
 	char _f[20-2*sizeof(long)-sizeof(int)];	/* Padding: libc5 uses this.. */
 };
 
+extern void out_of_line_bug(void) ATTRIB_NORET;
+
 #endif
include/asm-i386/pci.h renamed from include/asm-i386/pci.h~inline-BUG +10 -10
@@ -73,7 +73,7 @@ static inline dma_addr_t pci_map_single(
 					size_t size, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	flush_write_buffers();
 	return virt_to_bus(ptr);
 }
@@ -89,7 +89,7 @@ static inline void pci_unmap_single(stru
 				    size_t size, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	/* Nothing to do */
 }
 
@@ -101,7 +101,7 @@ static inline dma_addr_t pci_map_page(st
 				      unsigned long offset, size_t size, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 
 	return (page - mem_map) * PAGE_SIZE + offset;
 }
@@ -110,7 +110,7 @@ static inline void pci_unmap_page(struct
 				  size_t size, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	/* Nothing to do */
 }
 
@@ -143,16 +143,16 @@ static inline int pci_map_sg(struct pci_
 	int i;
 
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
  
  	/*
  	 * temporary 2.4 hack
  	 */
  	for (i = 0; i < nents; i++ ) {
  		if (sg[i].address && sg[i].page)
- 			BUG();
+ 			out_of_line_bug();
  		else if (!sg[i].address && !sg[i].page)
- 			BUG();
+ 			out_of_line_bug();
  
  		if (sg[i].address)
  			sg[i].dma_address = virt_to_bus(sg[i].address);
@@ -172,7 +172,7 @@ static inline void pci_unmap_sg(struct p
 				int nents, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	/* Nothing to do */
 }
 
@@ -190,7 +190,7 @@ static inline void pci_dma_sync_single(s
 				       size_t size, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	flush_write_buffers();
 }
 
@@ -205,7 +205,7 @@ static inline void pci_dma_sync_sg(struc
 				   int nelems, int direction)
 {
 	if (direction == PCI_DMA_NONE)
-		BUG();
+		out_of_line_bug();
 	flush_write_buffers();
 }
 
include/asm-i386/smplock.h renamed from include/asm-i386/smplock.h~inline-BUG +1 -1
@@ -59,7 +59,7 @@ static __inline__ void lock_kernel(void)
 static __inline__ void unlock_kernel(void)
 {
 	if (current->lock_depth < 0)
-		BUG();
+		out_of_line_bug();
 #if 1
 	if (--current->lock_depth < 0)
 		spin_unlock(&kernel_flag);
include/linux/dcache.h renamed from include/linux/dcache.h~inline-BUG +1 -4
@@ -242,11 +242,8 @@ extern char * __d_path(struct dentry *, 
  
 static __inline__ struct dentry * dget(struct dentry *dentry)
 {
-	if (dentry) {
-		if (!atomic_read(&dentry->d_count))
-			BUG();
+	if (dentry)
 		atomic_inc(&dentry->d_count);
-	}
 	return dentry;
 }
 
include/linux/file.h renamed from include/linux/file.h~inline-BUG +1 -24
@@ -71,30 +71,7 @@ static inline void put_unused_fd(unsigne
 	write_unlock(&files->file_lock);
 }
 
-/*
- * Install a file pointer in the fd array.  
- *
- * The VFS is full of places where we drop the files lock between
- * setting the open_fds bitmap and installing the file in the file
- * array.  At any such point, we are vulnerable to a dup2() race
- * installing a file in the array before us.  We need to detect this and
- * fput() the struct file we are about to overwrite in this case.
- *
- * It should never happen - if we allow dup2() do it, _really_ bad things
- * will follow.
- */
-
-static inline void fd_install(unsigned int fd, struct file * file)
-{
-	struct files_struct *files = current->files;
-	
-	write_lock(&files->file_lock);
-	if (files->fd[fd])
-		BUG();
-	files->fd[fd] = file;
-	write_unlock(&files->file_lock);
-}
-
+void fd_install(unsigned int fd, struct file * file);
 void put_files_struct(struct files_struct *fs);
 
 #endif /* __LINUX_FILE_H */
include/linux/nfs_fs.h renamed from include/linux/nfs_fs.h~inline-BUG +1 -1
@@ -168,7 +168,7 @@ nfs_file_cred(struct file *file)
 	struct rpc_cred *cred = (struct rpc_cred *)(file->private_data);
 #ifdef RPC_DEBUG
 	if (cred && cred->cr_magic != RPCAUTH_CRED_MAGIC)
-		BUG();
+		out_of_line_bug();
 #endif
 	return cred;
 }
include/linux/quotaops.h renamed from include/linux/quotaops.h~inline-BUG +1 -6
@@ -40,8 +40,6 @@ extern int  dquot_transfer(struct inode 
 
 static __inline__ void DQUOT_INIT(struct inode *inode)
 {
-	if (!inode->i_sb)
-		BUG();
 	lock_kernel();
 	if (sb_any_quota_enabled(inode->i_sb) && !IS_NOQUOTA(inode))
 		inode->i_sb->dq_op->initialize(inode, -1);
@@ -51,11 +49,8 @@ static __inline__ void DQUOT_INIT(struct
 static __inline__ void DQUOT_DROP(struct inode *inode)
 {
 	lock_kernel();
-	if (IS_QUOTAINIT(inode)) {
-		if (!inode->i_sb)
-			BUG();
+	if (IS_QUOTAINIT(inode))
 		inode->i_sb->dq_op->drop(inode);	/* Ops must be set when there's any quota... */
-	}
 	unlock_kernel();
 }
 
include/linux/sched.h renamed from include/linux/sched.h~inline-BUG +0 -1
@@ -896,7 +896,6 @@ static inline int task_on_runqueue(struc
 
 static inline void unhash_process(struct task_struct *p)
 {
-	if (task_on_runqueue(p)) BUG();
 	write_lock_irq(&tasklist_lock);
 	nr_threads--;
 	unhash_pid(p);
include/linux/highmem.h renamed from include/linux/highmem.h~inline-BUG +0 -2
@@ -63,8 +63,6 @@ static inline void memclear_highpage_flu
 {
 	char *kaddr;
 
-	if (offset + size > PAGE_SIZE)
-		BUG();
 	kaddr = kmap(page);
 	memset(kaddr + offset, 0, size);
 	flush_dcache_page(page);
include/linux/skbuff.h renamed from include/linux/skbuff.h~inline-BUG +4 -6
@@ -756,9 +756,9 @@ static inline int skb_headlen(const stru
 	return skb->len - skb->data_len;
 }
 
-#define SKB_PAGE_ASSERT(skb) do { if (skb_shinfo(skb)->nr_frags) BUG(); } while (0)
-#define SKB_FRAG_ASSERT(skb) do { if (skb_shinfo(skb)->frag_list) BUG(); } while (0)
-#define SKB_LINEAR_ASSERT(skb) do { if (skb_is_nonlinear(skb)) BUG(); } while (0)
+#define SKB_PAGE_ASSERT(skb) do { if (skb_shinfo(skb)->nr_frags) out_of_line_bug(); } while (0)
+#define SKB_FRAG_ASSERT(skb) do { if (skb_shinfo(skb)->frag_list) out_of_line_bug(); } while (0)
+#define SKB_LINEAR_ASSERT(skb) do { if (skb_is_nonlinear(skb)) out_of_line_bug(); } while (0)
 
 /*
  *	Add data to an sk_buff
@@ -825,8 +825,6 @@ static inline unsigned char *skb_push(st
 static inline char *__skb_pull(struct sk_buff *skb, unsigned int len)
 {
 	skb->len-=len;
-	if (skb->len < skb->data_len)
-		BUG();
 	return 	skb->data+=len;
 }
 
@@ -1094,7 +1092,7 @@ static inline void *kmap_skb_frag(const 
 {
 #ifdef CONFIG_HIGHMEM
 	if (in_irq())
-		BUG();
+		out_of_line_bug();
 
 	local_bh_disable();
 #endif
include/asm-i386/highmem.h renamed from include/asm-i386/highmem.h~inline-BUG +4 -4
@@ -62,7 +62,7 @@ extern void FASTCALL(kunmap_high(struct 
 static inline void *kmap(struct page *page)
 {
 	if (in_interrupt())
-		BUG();
+		out_of_line_bug();
 	if (page < highmem_start_page)
 		return page_address(page);
 	return kmap_high(page);
@@ -71,7 +71,7 @@ static inline void *kmap(struct page *pa
 static inline void kunmap(struct page *page)
 {
 	if (in_interrupt())
-		BUG();
+		out_of_line_bug();
 	if (page < highmem_start_page)
 		return;
 	kunmap_high(page);
@@ -95,7 +95,7 @@ static inline void *kmap_atomic(struct p
 	vaddr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
 #if HIGHMEM_DEBUG
 	if (!pte_none(*(kmap_pte-idx)))
-		BUG();
+		out_of_line_bug();
 #endif
 	set_pte(kmap_pte-idx, mk_pte(page, kmap_prot));
 	__flush_tlb_one(vaddr);
@@ -113,7 +113,7 @@ static inline void kunmap_atomic(void *k
 		return;
 
 	if (vaddr != __fix_to_virt(FIX_KMAP_BEGIN+idx))
-		BUG();
+		out_of_line_bug();
 
 	/*
 	 * force other mappings to Oops if they'll try to access
include/net/tcp.h renamed from include/net/tcp.h~inline-BUG +0 -2
@@ -1329,8 +1329,6 @@ static __inline__ int tcp_prequeue(struc
 		if (tp->ucopy.memory > sk->rcvbuf) {
 			struct sk_buff *skb1;
 
-			if (sk->lock.users) BUG();
-
 			while ((skb1 = __skb_dequeue(&tp->ucopy.prequeue)) != NULL) {
 				sk->backlog_rcv(sk, skb1);
 				NET_INC_STATS_BH(TCPPrequeueDropped);
fs/open.c renamed from fs/open.c~inline-BUG +24 -0
@@ -71,6 +71,30 @@ out:
 	return error;
 }
 
+/*
+ * Install a file pointer in the fd array.  
+ *
+ * The VFS is full of places where we drop the files lock between
+ * setting the open_fds bitmap and installing the file in the file
+ * array.  At any such point, we are vulnerable to a dup2() race
+ * installing a file in the array before us.  We need to detect this and
+ * fput() the struct file we are about to overwrite in this case.
+ *
+ * It should never happen - if we allow dup2() do it, _really_ bad things
+ * will follow.
+ */
+
+void fd_install(unsigned int fd, struct file * file)
+{
+	struct files_struct *files = current->files;
+	
+	write_lock(&files->file_lock);
+	if (files->fd[fd])
+		BUG();
+	files->fd[fd] = file;
+	write_unlock(&files->file_lock);
+}
+
 int do_truncate(struct dentry *dentry, loff_t length)
 {
 	struct inode *inode = dentry->d_inode;
kernel/ksyms.c renamed from kernel/ksyms.c~inline-BUG +2 -0
@@ -164,6 +164,7 @@ EXPORT_SYMBOL(mark_buffer_dirty);
 EXPORT_SYMBOL(set_buffer_async_io); /* for reiserfs_writepage */
 EXPORT_SYMBOL(__mark_buffer_dirty);
 EXPORT_SYMBOL(__mark_inode_dirty);
+EXPORT_SYMBOL(fd_install);
 EXPORT_SYMBOL(get_empty_filp);
 EXPORT_SYMBOL(init_private_file);
 EXPORT_SYMBOL(filp_open);
@@ -452,6 +453,7 @@ EXPORT_SYMBOL(nr_running);
 
 /* misc */
 EXPORT_SYMBOL(panic);
+EXPORT_SYMBOL(out_of_line_bug);
 EXPORT_SYMBOL(sprintf);
 EXPORT_SYMBOL(snprintf);
 EXPORT_SYMBOL(sscanf);
kernel/panic.c renamed from kernel/panic.c~inline-BUG +18 -0
@@ -124,3 +124,21 @@ const char *print_tainted()
 }
 
 int tainted = 0;
+
+/*
+ * A BUG() call in an inline function in a header should be avoided,
+ * because it can seriously bloat the kernel.  So here we have
+ * helper functions.
+ * We lose the BUG()-time file-and-line info this way, but it's
+ * usually not very useful from an inline anyway.  The backtrace
+ * tells us what we want to know.
+ */
+
+void out_of_line_bug(void)
+{
+	BUG();
+
+	/* Satisfy __attribute__((noreturn)) */
+	for ( ; ; )
+		;
+}

-

#1 lore raw
Quoted 4 lines · On Tue, 19 Mar 2002 22:11:48 -0800 Andrew Morton <[email protected]> wrote:

This is the result of a search-and-destroy mission against oft-repeated strings in the kernel text. These come about due to the toolchain's inability to common up strings between compilation units.

The name is horrible. BUG() stands out: perhaps "BUG_LITE()"?

And I'm not sure DaveM'll appreciate this:

include/linux/skbuff.h static inline unsigned char *skb_push(st · line 829

Quoted 7 lines

static inline char *__skb_pull(struct sk_buff *skb, unsigned int len) {

include/linux/skbuff.h renamed from include/linux/skbuff.h~inline-BUG +0 -2
 	skb->len-=len;
-	if (skb->len < skb->data_len)
-		BUG();
 	return 	skb->data+=len;
 }

Rusty.

Signature · 1 line
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
#2 lore raw

From: Rusty Russell <[email protected]>
Date: Mon, 25 Mar 2002 16:56:05 +1100

And I'm not sure DaveM'll appreciate this:

include/linux/skbuff.h static inline unsigned char *skb_push(st · line 829

Quoted 7 lines

static inline char *__skb_pull(struct sk_buff *skb, unsigned int len) {

include/linux/skbuff.h renamed from include/linux/skbuff.h~inline-BUG +0 -2
 	skb->len-=len;
-	if (skb->len < skb->data_len)
-		BUG();
 	return 	skb->data+=len;
 }

Rusty's right, I definitely won't take this, it catches problems %99 of the time in the place that causes it.

#3 lore raw
Quoted 13 lines · David S. Miller
   From: Rusty Russell <[email protected]>
   Date: Mon, 25 Mar 2002 16:56:05 +1100

   And I'm not sure DaveM'll appreciate this:
   
   >  static inline char *__skb_pull(struct sk_buff *skb, unsigned int len)
   >  {
   >  	skb->len-=len;
   > -	if (skb->len < skb->data_len)
   > -		BUG();
   >  	return 	skb->data+=len;
   >  }
Quoted 2 lines

Rusty's right, I definitely won't take this, it catches problems %99 of the time in the place that causes it.

But, it makes the kernel smaller. And faster! ;-)

Signature · 3 lines
bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.
#4 lore raw
In reply to Rusty Russell
Quoted 12 lines · Rusty Russell
Quoted 4 lines · On Tue, 19 Mar 2002 22:11:48 -0800 Andrew Morton <[email protected]> wrote:

This is the result of a search-and-destroy mission against oft-repeated strings in the kernel text. These come about due to the toolchain's inability to common up strings between compilation units.

The name is horrible. BUG() stands out: perhaps "BUG_LITE()"?

And I'm not sure DaveM'll appreciate this:

Quoted 7 lines

include/linux/skbuff.h static inline unsigned char *skb_push(st · line 829

Quoted 7 lines

static inline char *__skb_pull(struct sk_buff *skb, unsigned int len) {

include/linux/skbuff.h renamed from include/linux/skbuff.h~inline-BUG +0 -2
 	skb->len-=len;
-	if (skb->len < skb->data_len)
-		BUG();
 	return 	skb->data+=len;
 }

Andrew,

I've just readded all asserts which you removed... if you really want to remove any of those, please prove me that they are useless.

#5 lore raw
In reply to Rusty Russell
Quoted 3 lines · Marcelo Tosatti

I've just readded all asserts which you removed... if you really want to remove any of those, please prove me that they are useless.

I grepped a year's lkml traffic - nobody is hitting any of them...

The quotaops.h checks were useless:

	if (pointer == NULL)
		BUG();
	dereference(pointer);

The others can become calls to out_of_line_bug() if you want. That's dget(), unhash_process(), memclear_highpage_flush(), __skb_pull() and tcp_prequeue()

-

#6 lore raw
Quoted 7 lines · Andrew Morton
Quoted 3 lines · Marcelo Tosatti

I've just readded all asserts which you removed... if you really want to remove any of those, please prove me that they are useless.

I grepped a year's lkml traffic - nobody is hitting any of them...

Anyway, they may catch bugs introduced by new modifications...

Quoted 8 lines

The quotaops.h checks were useless:

	if (pointer == NULL)
		BUG();
	dereference(pointer);

The others can become calls to out_of_line_bug() if you want.

Please do, thanks.

#7 lore raw
In reply to Andrew Morton
Quoted 10 lines · Rusty Russell
Quoted 4 lines · On Tue, 19 Mar 2002 22:11:48 -0800 Andrew Morton <[email protected]> wrote:

This is the result of a search-and-destroy mission against oft-repeated strings in the kernel text. These come about due to the toolchain's inability to common up strings between compilation units.

The name is horrible. BUG() stands out: perhaps "BUG_LITE()"?

out_of_line_bug()? It's a bit ornate I guess, but it tells you what it does when you look at it.

Quoted 9 lines

And I'm not sure DaveM'll appreciate this:

Quoted 7 lines

static inline char *__skb_pull(struct sk_buff *skb, unsigned int len)

 {
      skb->len-=len;
include/linux/skbuff.h renamed from include/linux/skbuff.h~inline-BUG +0 -2
-     if (skb->len < skb->data_len)
-             BUG();
      return  skb->data+=len;
 }

OK, we can out that back (out-of-line) for -pre6.

-

#8 lore raw

In message <[email protected]> you write:

Quoted 10 lines
Quoted 9 lines · Rusty Russell
Quoted 4 lines · On Tue, 19 Mar 2002 22:11:48 -0800 Andrew Morton <[email protected]> wrote:

This is the result of a search-and-destroy mission against oft-repeated strings in the kernel text. These come about due to the toolchain's inability to common up strings between compilation units.

Quoted 4 lines
Quoted 1 line

The name is horrible. BUG() stands out: perhaps "BUG_LITE()"?

out_of_line_bug()? It's a bit ornate I guess, but it tells you what it does when you look at it.

I like BUG() because it stands out in a fast code scan. And the shouting prevents people from sprinkling them everywhere like a drunken soldier (although there are still some people who seem to want to BUG before ops which would oops anyway...)

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.