remove dentry_open::file_ra_init_state() duplicated memset was Re: kernbench on 512p

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Thu, Aug 19, 2004 at 12:16:33PM -0400, Jesse Barnes wrote:

> Here's the kernel profile.  It would be nice if the patch to show which lock 
> is contended got included.  I think it was discussed awhile back, but I don't 
> have one against the newly merged profiling code.
> 
> [root@ascender root]# readprofile -m System.map | sort -nr | head -30
> 208218076 total                                     30.1677
> 90036167 ia64_pal_call_static                     468938.3698
> 88140492 default_idle                             229532.5312
> 10312592 ia64_save_scratch_fpregs                 161134.2500
> 10306777 ia64_load_scratch_fpregs                 161043.3906
> 8723555 ia64_spinlock_contention                 90870.3646
> 121385 rcu_check_quiescent_state                316.1068
>  40464 file_move                                180.6429
>  32374 file_kill                                144.5268
>  25316 atomic_dec_and_lock                       98.8906
>  24814 clear_page                               155.0875
>  17709 file_ra_state_init                       110.6813

This is pretty high. Andi says its probably not due to this function 
itself, but for some other reason, like cache trashing caused, accidentally, 
by it.

But anyway, investigation of file_ra_state_init() shows that its called 
from dentry_open()... it does a memset(ra, 0, sizeof(*ra)), which is not needed, 
because get_empty_filp() already memset's the whole "struct file" to 0.

So this patch creates a __file_ra_state_init() function, which initializes
the file_ra_state fields, without the memset. 

file_ra_state_init() does the memset + its __ counterpart. 

Pretty straightforward and simple.

Against 2.6.8.1-mm2, Andrew, please apply.

Jesse, if you could give it a shot and rerun the profile.

diff -Nur --exclude='*.orig' linux-2.6.8.orig/fs/open.c linux-2.6.8/fs/open.c
--- linux-2.6.8.orig/fs/open.c	2004-08-14 02:36:13.000000000 -0300
+++ linux-2.6.8/fs/open.c	2004-08-19 22:38:48.000000000 -0300
@@ -803,7 +803,7 @@
 	}
 	f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
 
-	file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
+	__file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
 
 	/* NB: we're sure to have correct a_ops only after f_op->open */
 	if (f->f_flags & O_DIRECT) {
diff -Nur --exclude='*.orig' linux-2.6.8.orig/include/linux/fs.h linux-2.6.8/include/linux/fs.h
--- linux-2.6.8.orig/include/linux/fs.h	2004-08-19 22:35:49.000000000 -0300
+++ linux-2.6.8/include/linux/fs.h	2004-08-19 22:39:55.000000000 -0300
@@ -1520,7 +1520,10 @@
 				    struct file_ra_state *, struct file *,
 				    loff_t *, read_descriptor_t *, read_actor_t);
 extern void
+__file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
+extern void
 file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
+
 extern ssize_t generic_file_direct_IO(int rw, struct kiocb *iocb,
 	const struct iovec *iov, loff_t offset, unsigned long nr_segs);
 extern ssize_t generic_file_readv(struct file *filp, const struct iovec *iov, 
diff -Nur --exclude='*.orig' linux-2.6.8.orig/mm/readahead.c linux-2.6.8/mm/readahead.c
--- linux-2.6.8.orig/mm/readahead.c	2004-08-19 22:35:50.000000000 -0300
+++ linux-2.6.8/mm/readahead.c	2004-08-19 22:37:49.000000000 -0300
@@ -30,13 +30,22 @@
 /*
  * Initialise a struct file's readahead state
  */
+
 void
-file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
+__file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
 {
-	memset(ra, 0, sizeof(*ra));
 	ra->ra_pages = mapping->backing_dev_info->ra_pages;
 	ra->average = ra->ra_pages / 2;
 }
+
+void
+file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
+{
+	memset(ra, 0, sizeof(*ra));
+	__file_ra_state_init(ra, mapping);
+}
+
+EXPORT_SYMBOL(__file_ra_state_init);
 EXPORT_SYMBOL(file_ra_state_init);
 
 /*