lkml.org 
[lkml]   [1998]   [Dec]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] optimization and NCPFS bugfix
The following tinny attached patch provides:

1. An increddible saving to the kernel size of more then 3756 bytes
by just removing some bogous extern inline optimization attempt.

2. Proper unlocking in the NCPFS during unmounts (discovered during the
above).

3. Hide of file lock list from external access (made possible by the
above).

4. Politically correct (SHORTER without lost of information!) panic
messages in lock handling
(discovered during point 2.)

Plase appy it. (Whoever likes...)
The comments are mainly intended for easier patch understanding and
may well be removed if needed.

I suspect that there are many many similiar code qulity problems
scattered through the whole kernel which are due to bogous inlining of
functions, which incurr function calls anyway.

The patch sustained a diff -urN on two different kernels and I'm
actually
writing this message with it running.
BTW. it's just correct by arithmetics...

MarcinBinary files linux/drivers/char/.mem.c.swp and linux-current/drivers/char/.mem.c.swp differ
Binary files linux/fs/.file_table.c.swp and linux-current/fs/.file_table.c.swp differ
diff -Nur linux/fs/file_table.c linux-current/fs/file_table.c
--- linux/fs/file_table.c Thu Nov 19 21:22:55 1998
+++ linux-current/fs/file_table.c Wed Dec 23 19:06:15 1998
@@ -20,7 +20,7 @@
/* Free list management, if you are here you must have f_count == 0 */
static struct file * free_filps = NULL;

-void insert_file_free(struct file *file)
+static void insert_file_free(struct file *file)
{
if((file->f_next = free_filps) != NULL)
free_filps->f_pprev = &file->f_next;
@@ -40,6 +40,15 @@
file->f_pprev = &inuse_filps;
}

+/* It does not matter which list it is on. */
+static inline void remove_filp(struct file *file)
+{
+ if(file->f_next)
+ file->f_next->f_pprev = file->f_pprev;
+ *file->f_pprev = file->f_next;
+}
+
+
void __init file_table_init(void)
{
filp_cache = kmem_cache_create("filp", sizeof(struct file),
@@ -115,4 +124,26 @@
return filp->f_op->open(dentry->d_inode, filp);
else
return 0;
+}
+
+void fput(struct file *file)
+{
+ int count = file->f_count-1;
+
+ if (!count) {
+ locks_remove_flock(file);
+ __fput(file);
+ file->f_count = 0;
+ remove_filp(file);
+ insert_file_free(file);
+ } else
+ file->f_count = count;
+}
+
+void put_filp(struct file *file)
+{
+ if(--file->f_count == 0) {
+ remove_filp(file);
+ insert_file_free(file);
+ }
}
diff -Nur linux/fs/locks.c linux-current/fs/locks.c
--- linux/fs/locks.c Thu Nov 19 21:22:55 1998
+++ linux-current/fs/locks.c Wed Dec 23 19:20:01 1998
@@ -138,7 +138,7 @@
static void locks_delete_block(struct file_lock *blocker, struct file_lock *waiter);
static void locks_wake_up_blocks(struct file_lock *blocker, unsigned int wait);

-struct file_lock *file_lock_table = NULL;
+static struct file_lock *file_lock_table = NULL;

/* Allocate a new lock, and initialize its fields from fl.
* The lock is not inserted into any lists until locks_insert_lock() or
@@ -154,10 +154,10 @@
static inline void locks_free_lock(struct file_lock *fl)
{
if (waitqueue_active(&fl->fl_wait))
- panic("Aarggh: attempting to free lock with active wait queue - shoot Andy");
+ panic("Attempting to free lock with active wait queue");

if (fl->fl_nextblock != NULL || fl->fl_prevblock != NULL)
- panic("Aarggh: attempting to free lock with active block list - shoot Andy");
+ panic("Attempting to free lock with active block list");

kfree(fl);
return;
diff -Nur linux/fs/ncpfs/inode.c linux-current/fs/ncpfs/inode.c
--- linux/fs/ncpfs/inode.c Sat Apr 4 19:45:16 1998
+++ linux-current/fs/ncpfs/inode.c Wed Dec 23 19:06:10 1998
@@ -382,7 +382,12 @@
out_no_server:
printk(KERN_ERR "ncp_read_super: could not alloc ncp_server\n");
out_unlock:
- put_filp(ncp_filp);
+ /* 23/12/1998 Marcin Dalecki <dalecki@cs.net.pl>:
+ *
+ * The previously used put_filp(ncp_filp); was bogous, since
+ * it doesn't proper unlocking.
+ */
+ fput(ncp_filp);
unlock_super(sb);
goto out;

Binary files linux/include/linux/.file.h.swp and linux-current/include/linux/.file.h.swp differ
diff -Nur linux/include/linux/file.h linux-current/include/linux/file.h
--- linux/include/linux/file.h Mon Aug 24 22:56:09 1998
+++ linux-current/include/linux/file.h Wed Dec 23 19:21:45 1998
@@ -6,7 +6,6 @@
#define __LINUX_FILE_H

extern void __fput(struct file *);
-extern void insert_file_free(struct file *file);

/*
* Check whether the specified task has the fd open. Since the task
@@ -50,34 +49,23 @@
current->files->fd[fd] = file;
}

-/* It does not matter which list it is on. */
-extern inline void remove_filp(struct file *file)
-{
- if(file->f_next)
- file->f_next->f_pprev = file->f_pprev;
- *file->f_pprev = file->f_next;
-}
-
-extern inline void fput(struct file *file)
-{
- int count = file->f_count-1;
-
- if (!count) {
- locks_remove_flock(file);
- __fput(file);
- file->f_count = 0;
- remove_filp(file);
- insert_file_free(file);
- } else
- file->f_count = count;
-}
-
-extern inline void put_filp(struct file *file)
-{
- if(--file->f_count == 0) {
- remove_filp(file);
- insert_file_free(file);
- }
-}
+/*
+ * 23/12/1998 Marcin Dalecki <dalecki@cs.net.pl>:
+ *
+ * Since those functions where calling other functions, it was compleatly
+ * bogous to make them all "extern inline".
+ *
+ * The removal of this pseudo optimization saved me scandaleous:
+ *
+ * 3756 (i386 arch)
+ *
+ * precious bytes from my kernel, even without counting all the code compiled
+ * as module!
+ *
+ * I suspect there are many other similiar "optimizations" across the
+ * kernel...
+ */
+extern void fput(struct file *file);
+extern void put_filp(struct file *file);

#endif
diff -Nur linux/include/linux/fs.h linux-current/include/linux/fs.h
--- linux/include/linux/fs.h Wed Dec 23 15:36:21 1998
+++ linux-current/include/linux/fs.h Wed Dec 23 19:11:56 1998
@@ -466,8 +466,6 @@
} fl_u;
};

-extern struct file_lock *file_lock_table;
-
#include <linux/fcntl.h>

extern int fcntl_getlk(unsigned int fd, struct flock *l);
diff -Nur linux/kernel/ksyms.c linux-current/kernel/ksyms.c
--- linux/kernel/ksyms.c Wed Dec 23 15:18:51 1998
+++ linux-current/kernel/ksyms.c Wed Dec 23 19:11:55 1998
@@ -129,7 +129,7 @@
EXPORT_SYMBOL(__mark_inode_dirty);
EXPORT_SYMBOL(get_empty_filp);
EXPORT_SYMBOL(init_private_file);
-EXPORT_SYMBOL(insert_file_free);
+EXPORT_SYMBOL(fput);
EXPORT_SYMBOL(check_disk_change);
EXPORT_SYMBOL(invalidate_buffers);
EXPORT_SYMBOL(invalidate_inodes);
@@ -156,12 +156,10 @@
EXPORT_SYMBOL(generic_file_write);
EXPORT_SYMBOL(generic_file_mmap);
EXPORT_SYMBOL(generic_readpage);
-EXPORT_SYMBOL(file_lock_table);
EXPORT_SYMBOL(posix_lock_file);
EXPORT_SYMBOL(posix_test_lock);
EXPORT_SYMBOL(posix_block_lock);
EXPORT_SYMBOL(posix_unblock_lock);
-EXPORT_SYMBOL(locks_remove_flock);
EXPORT_SYMBOL(dput);
EXPORT_SYMBOL(get_cached_page);
EXPORT_SYMBOL(put_cached_page);
Binary files linux/linux.tar.gz and linux-current/linux.tar.gz differ
\
 
 \ /
  Last update: 2005-03-22 13:46    [from the cache]
©2003-2011 Jasper Spaans