public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 0/4] Series to allow a "const" file_operations struct
@ 2006-01-06 21:45 Arjan van de Ven
  2006-01-06 21:46 ` [patch 1/4] fix some f_ops abuse in acpi Arjan van de Ven
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm

Hi,

this series allows drivers to have "const" file_operations, by making
the f_ops field in the inode const. This has another benefit, there have
been several bugs where code accidentally wrote to the ->f_ops
structure, forgetting that it's a shared structure. One of those bugs
got fixed in november for example (as a result of these patches),
another one is fixed in this series. 

Greetings,
   Arjan van de Ven


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [patch 1/4]  fix some f_ops abuse in acpi
  2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
@ 2006-01-06 21:46 ` Arjan van de Ven
  2006-01-06 21:47 ` [patch 2/4] fix input layer f_ops abuse Arjan van de Ven
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:46 UTC (permalink / raw)
  To: linux-kernel; +Cc: len.brown, akpm

From: Arjan van de Ven <arjan@infradead.org>

The ACPI layer writes to acpi_processor_limit_fops.write at runtime, while
it in reality is a compile time constant. Move this to the struct definition
instead.
Similar for acpi_video_bus_POST_fops.write and friends, but keep doing those 
at runtime to avoid prototype-hell.

Signed-off-by: Arjan van de Ven <arjan@infradead.org>
CC: Len Brown <len.brown@intel.com>

Index: linux-2.6.15/drivers/acpi/processor_core.c
 drivers/acpi/processor_core.c       |    2 --
 drivers/acpi/processor_perflib.c    |    2 +-
 drivers/acpi/processor_thermal.c    |    1 +
 drivers/acpi/processor_throttling.c |    1 +
 drivers/acpi/video.c                |    8 ++++----
 5 files changed, 7 insertions(+), 7 deletions(-)

Index: linux-2.6.15/drivers/acpi/processor_core.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/processor_core.c
+++ linux-2.6.15/drivers/acpi/processor_core.c
@@ -357,7 +357,6 @@ static int acpi_processor_add_fs(struct 
 				  ACPI_PROCESSOR_FILE_THROTTLING));
 	else {
 		entry->proc_fops = &acpi_processor_throttling_fops;
-		entry->proc_fops->write = acpi_processor_write_throttling;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
@@ -372,7 +371,6 @@ static int acpi_processor_add_fs(struct 
 				  ACPI_PROCESSOR_FILE_LIMIT));
 	else {
 		entry->proc_fops = &acpi_processor_limit_fops;
-		entry->proc_fops->write = acpi_processor_write_limit;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
Index: linux-2.6.15/drivers/acpi/processor_thermal.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/processor_thermal.c
+++ linux-2.6.15/drivers/acpi/processor_thermal.c
@@ -394,6 +394,7 @@ ssize_t acpi_processor_write_limit(struc
 struct file_operations acpi_processor_limit_fops = {
 	.open = acpi_processor_limit_open_fs,
 	.read = seq_read,
+	.write = acpi_processor_write_limit,
 	.llseek = seq_lseek,
 	.release = single_release,
 };
Index: linux-2.6.15/drivers/acpi/processor_throttling.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/processor_throttling.c
+++ linux-2.6.15/drivers/acpi/processor_throttling.c
@@ -337,6 +337,7 @@ ssize_t acpi_processor_write_throttling(
 struct file_operations acpi_processor_throttling_fops = {
 	.open = acpi_processor_throttling_open_fs,
 	.read = seq_read,
+	.write = acpi_processor_write_throttling,
 	.llseek = seq_lseek,
 	.release = single_release,
 };
Index: linux-2.6.15/drivers/acpi/video.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/video.c
+++ linux-2.6.15/drivers/acpi/video.c
@@ -920,8 +920,8 @@ static int acpi_video_device_add_fs(stru
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				  "Unable to create 'state' fs entry\n"));
 	else {
+		acpi_video_device_state_fops.write = acpi_video_device_write_state;
 		entry->proc_fops = &acpi_video_device_state_fops;
-		entry->proc_fops->write = acpi_video_device_write_state;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
@@ -934,8 +934,8 @@ static int acpi_video_device_add_fs(stru
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				  "Unable to create 'brightness' fs entry\n"));
 	else {
+		acpi_video_device_brightness_fops.write = acpi_video_device_write_brightness;
 		entry->proc_fops = &acpi_video_device_brightness_fops;
-		entry->proc_fops->write = acpi_video_device_write_brightness;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
@@ -1239,8 +1239,8 @@ static int acpi_video_bus_add_fs(struct 
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				  "Unable to create 'POST' fs entry\n"));
 	else {
+		acpi_video_bus_POST_fops.write = acpi_video_bus_write_POST;
 		entry->proc_fops = &acpi_video_bus_POST_fops;
-		entry->proc_fops->write = acpi_video_bus_write_POST;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
@@ -1253,8 +1253,8 @@ static int acpi_video_bus_add_fs(struct 
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
 				  "Unable to create 'DOS' fs entry\n"));
 	else {
+		acpi_video_bus_DOS_fops.write = acpi_video_bus_write_DOS;
 		entry->proc_fops = &acpi_video_bus_DOS_fops;
-		entry->proc_fops->write = acpi_video_bus_write_DOS;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}
Index: linux-2.6.15/drivers/acpi/processor_perflib.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/processor_perflib.c
+++ linux-2.6.15/drivers/acpi/processor_perflib.c
@@ -520,8 +520,8 @@ static void acpi_cpufreq_add_file(struct
 				  "Unable to create '%s' fs entry\n",
 				  ACPI_PROCESSOR_FILE_PERFORMANCE));
 	else {
+		acpi_processor_perf_fops.write = acpi_processor_write_performance;
 		entry->proc_fops = &acpi_processor_perf_fops;
-		entry->proc_fops->write = acpi_processor_write_performance;
 		entry->data = acpi_driver_data(device);
 		entry->owner = THIS_MODULE;
 	}



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [patch 2/4] fix input layer f_ops abuse
  2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
  2006-01-06 21:46 ` [patch 1/4] fix some f_ops abuse in acpi Arjan van de Ven
@ 2006-01-06 21:47 ` Arjan van de Ven
  2006-01-06 23:12   ` Dmitry Torokhov
  2006-01-06 21:49 ` [patch 3/4] fix cifs bugs wrt writing to f_ops Arjan van de Ven
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm

From: Arjan van de Ven <arjan@infradead.org>

The input layer has an assignment to a live ->fops, just after creating the
fops as a duplicate of another one. Just move this assignment a few lines up to avoid
the race and the assignment to a live fops

Signed-off-by: Arjan van de Ven <arjan@infradead.org>
 drivers/input/input.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.15/drivers/input/input.c
===================================================================
--- linux-2.6.15.orig/drivers/input/input.c
+++ linux-2.6.15/drivers/input/input.c
@@ -478,8 +478,8 @@ static int __init input_proc_init(void)
 
 	entry->owner = THIS_MODULE;
 	input_fileops = *entry->proc_fops;
+	input_fileops.poll = input_devices_poll;
 	entry->proc_fops = &input_fileops;
-	entry->proc_fops->poll = input_devices_poll;
 
 	entry = create_proc_read_entry("handlers", 0, proc_bus_input_dir, input_handlers_read, NULL);
 	if (!entry)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [patch 3/4] fix cifs bugs wrt writing to f_ops
  2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
  2006-01-06 21:46 ` [patch 1/4] fix some f_ops abuse in acpi Arjan van de Ven
  2006-01-06 21:47 ` [patch 2/4] fix input layer f_ops abuse Arjan van de Ven
@ 2006-01-06 21:49 ` Arjan van de Ven
  2006-01-06 21:51 ` [patch 4/4] Actually make the f_ops field const Arjan van de Ven
  2006-01-06 21:55 ` [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
  4 siblings, 0 replies; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:49 UTC (permalink / raw)
  To: linux-kernel; +Cc: sfrench, akpms

From: Arjan van de Ven <arjan@infradead.org>

patch 2ea55c01e0c5dfead8699484b0bae2a375b1f61c fixed CIFS clobbering the
global fops structure for some per mount setting, by duplicating and having
2 fops structs. However the write to the fops was left behind, which is a
NOP in practice (due to the fact that we KNOW the fops has that field set to
NULL already due to the duplication). So remove it... In addition, another
instance of the same bug was forgotten in november.

Signed-off-by: Arjan van de Ven <arjan@infradead.org>
CC: Steve French <sfrench@us.ibm.com>

 fs/cifs/readdir.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Index: linux-2.6.15/fs/cifs/readdir.c
===================================================================
--- linux-2.6.15.orig/fs/cifs/readdir.c
+++ linux-2.6.15/fs/cifs/readdir.c
@@ -214,8 +214,7 @@ static void fill_in_inode(struct inode *
 			tmp_inode->i_fop = &cifs_file_nobrl_ops;
 		else
 			tmp_inode->i_fop = &cifs_file_ops;
-		if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
-			tmp_inode->i_fop->lock = NULL;
+
 		tmp_inode->i_data.a_ops = &cifs_addr_ops;
 		if((cifs_sb->tcon) && (cifs_sb->tcon->ses) &&
 		   (cifs_sb->tcon->ses->server->maxBuf <
@@ -327,12 +326,18 @@ static void unix_fill_in_inode(struct in
 	if (S_ISREG(tmp_inode->i_mode)) {
 		cFYI(1, ("File inode"));
 		tmp_inode->i_op = &cifs_file_inode_ops;
-		if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO)
-			tmp_inode->i_fop = &cifs_file_direct_ops;
+
+		if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DIRECT_IO) {
+			if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
+				tmp_inode->i_fop = &cifs_file_direct_nobrl_ops;
+			else
+				tmp_inode->i_fop = &cifs_file_direct_ops;
+		
+		} else if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
+			tmp_inode->i_fop = &cifs_file_nobrl_ops;
 		else
 			tmp_inode->i_fop = &cifs_file_ops;
-		if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_BRL)
-			tmp_inode->i_fop->lock = NULL;
+
 		tmp_inode->i_data.a_ops = &cifs_addr_ops;
 		if((cifs_sb->tcon) && (cifs_sb->tcon->ses) &&
 		   (cifs_sb->tcon->ses->server->maxBuf < 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [patch 4/4] Actually make the f_ops field const
  2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
                   ` (2 preceding siblings ...)
  2006-01-06 21:49 ` [patch 3/4] fix cifs bugs wrt writing to f_ops Arjan van de Ven
@ 2006-01-06 21:51 ` Arjan van de Ven
  2006-01-07  8:00   ` Andrew Morton
  2006-01-06 21:55 ` [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
  4 siblings, 1 reply; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: aviro, akpm

From: Arjan van de Ven <arjan@infradead.org>

Mark the f_ops members of inodes as const, as well as fix the ripple-through
this causes by places that copy this f_ops and then "do stuff" with it.
(some of the "do stuff" is quite unpleasant..)
I've looked at several other alternatives for doing this but have not found one
that works well in the light that several pieces of code build their fops struct
dynamically at runtime.

Signed-off-by: Arjan van de Ven <arjan@infradead.org>
 drivers/char/drm/drm_stub.c         |    2 +-
 drivers/char/drm/i810_dma.c         |    2 +-
 drivers/char/drm/i830_dma.c         |    2 +-
 drivers/char/mem.c                  |    2 +-
 drivers/char/misc.c                 |    2 +-
 drivers/input/input.c               |    2 +-
 drivers/isdn/capi/kcapi_proc.c      |    2 +-
 drivers/media/dvb/dvb-core/dvbdev.c |    2 +-
 drivers/media/video/videodev.c      |    2 +-
 drivers/message/i2o/i2o_proc.c      |    2 +-
 drivers/oprofile/oprofilefs.c       |    6 +++---
 drivers/telephony/phonedev.c        |    2 +-
 drivers/usb/core/file.c             |    6 +++---
 fs/char_dev.c                       |    4 ++--
 fs/debugfs/inode.c                  |    2 +-
 fs/inode.c                          |    2 +-
 fs/nfsd/vfs.c                       |    2 +-
 fs/proc/generic.c                   |    2 +-
 fs/proc/internal.h                  |    2 +-
 fs/proc/proc_misc.c                 |    2 +-
 fs/select.c                         |    2 +-
 include/linux/cdev.h                |    4 ++--
 include/linux/debugfs.h             |    2 +-
 include/linux/fs.h                  |    6 +++---
 include/linux/input.h               |    2 +-
 include/linux/miscdevice.h          |    2 +-
 include/linux/oprofile.h            |    4 ++--
 include/linux/proc_fs.h             |    4 ++--
 include/linux/sound.h               |   12 ++++++------
 include/linux/sunrpc/stats.h        |    4 ++--
 include/linux/usb.h                 |    2 +-
 include/linux/videodev2.h           |    2 +-
 include/sound/core.h                |    2 +-
 net/sunrpc/rpc_pipe.c               |    2 +-
 net/sunrpc/stats.c                  |    4 ++--
 sound/core/init.c                   |    3 ++-
 sound/core/sound.c                  |    2 +-
 sound/sound_core.c                  |   22 +++++++++++-----------
 38 files changed, 66 insertions(+), 65 deletions(-)

Index: linux-2.6.15/drivers/char/drm/drm_stub.c
===================================================================
--- linux-2.6.15.orig/drivers/char/drm/drm_stub.c
+++ linux-2.6.15/drivers/char/drm/drm_stub.c
@@ -142,7 +142,7 @@ int drm_stub_open(struct inode *inode, s
 	drm_device_t *dev = NULL;
 	int minor = iminor(inode);
 	int err = -ENODEV;
-	struct file_operations *old_fops;
+	const struct file_operations *old_fops;
 
 	DRM_DEBUG("\n");
 
Index: linux-2.6.15/drivers/char/drm/i810_dma.c
===================================================================
--- linux-2.6.15.orig/drivers/char/drm/i810_dma.c
+++ linux-2.6.15/drivers/char/drm/i810_dma.c
@@ -127,7 +127,7 @@ static int i810_map_buffer(drm_buf_t * b
 	drm_device_t *dev = priv->head->dev;
 	drm_i810_buf_priv_t *buf_priv = buf->dev_private;
 	drm_i810_private_t *dev_priv = dev->dev_private;
-	struct file_operations *old_fops;
+	const struct file_operations *old_fops;
 	int retcode = 0;
 
 	if (buf_priv->currently_mapped == I810_BUF_MAPPED)
Index: linux-2.6.15/drivers/char/drm/i830_dma.c
===================================================================
--- linux-2.6.15.orig/drivers/char/drm/i830_dma.c
+++ linux-2.6.15/drivers/char/drm/i830_dma.c
@@ -129,7 +129,7 @@ static int i830_map_buffer(drm_buf_t * b
 	drm_device_t *dev = priv->head->dev;
 	drm_i830_buf_priv_t *buf_priv = buf->dev_private;
 	drm_i830_private_t *dev_priv = dev->dev_private;
-	struct file_operations *old_fops;
+	const struct file_operations *old_fops;
 	unsigned long virtual;
 	int retcode = 0;
 
Index: linux-2.6.15/drivers/char/mem.c
===================================================================
--- linux-2.6.15.orig/drivers/char/mem.c
+++ linux-2.6.15/drivers/char/mem.c
@@ -889,7 +889,7 @@ static const struct {
 	unsigned int		minor;
 	char			*name;
 	umode_t			mode;
-	struct file_operations	*fops;
+	const struct file_operations	*fops;
 } devlist[] = { /* list of minor devices */
 	{1, "mem",     S_IRUSR | S_IWUSR | S_IRGRP, &mem_fops},
 	{2, "kmem",    S_IRUSR | S_IWUSR | S_IRGRP, &kmem_fops},
Index: linux-2.6.15/drivers/char/misc.c
===================================================================
--- linux-2.6.15.orig/drivers/char/misc.c
+++ linux-2.6.15/drivers/char/misc.c
@@ -129,7 +129,7 @@ static int misc_open(struct inode * inod
 	int minor = iminor(inode);
 	struct miscdevice *c;
 	int err = -ENODEV;
-	struct file_operations *old_fops, *new_fops = NULL;
+	const struct file_operations *old_fops, *new_fops = NULL;
 	
 	down(&misc_sem);
 	
Index: linux-2.6.15/drivers/input/input.c
===================================================================
--- linux-2.6.15.orig/drivers/input/input.c
+++ linux-2.6.15/drivers/input/input.c
@@ -866,7 +866,7 @@ void input_unregister_handler(struct inp
 static int input_open_file(struct inode *inode, struct file *file)
 {
 	struct input_handler *handler = input_table[iminor(inode) >> 5];
-	struct file_operations *old_fops, *new_fops = NULL;
+	const struct file_operations *old_fops, *new_fops = NULL;
 	int err;
 
 	/* No load-on-demand here? */
Index: linux-2.6.15/drivers/isdn/capi/kcapi_proc.c
===================================================================
--- linux-2.6.15.orig/drivers/isdn/capi/kcapi_proc.c
+++ linux-2.6.15/drivers/isdn/capi/kcapi_proc.c
@@ -233,7 +233,7 @@ static struct file_operations proc_appls
 };
 
 static void
-create_seq_entry(char *name, mode_t mode, struct file_operations *f)
+create_seq_entry(char *name, mode_t mode, const struct file_operations *f)
 {
 	struct proc_dir_entry *entry;
 	entry = create_proc_entry(name, mode, NULL);
Index: linux-2.6.15/drivers/media/dvb/dvb-core/dvbdev.c
===================================================================
--- linux-2.6.15.orig/drivers/media/dvb/dvb-core/dvbdev.c
+++ linux-2.6.15/drivers/media/dvb/dvb-core/dvbdev.c
@@ -86,7 +86,7 @@ static int dvb_device_open(struct inode 
 
 	if (dvbdev && dvbdev->fops) {
 		int err = 0;
-		struct file_operations *old_fops;
+		const struct file_operations *old_fops;
 
 		file->private_data = dvbdev;
 		old_fops = file->f_op;
Index: linux-2.6.15/drivers/media/video/videodev.c
===================================================================
--- linux-2.6.15.orig/drivers/media/video/videodev.c
+++ linux-2.6.15/drivers/media/video/videodev.c
@@ -100,7 +100,7 @@ static int video_open(struct inode *inod
 	unsigned int minor = iminor(inode);
 	int err = 0;
 	struct video_device *vfl;
-	struct file_operations *old_fops;
+	const struct file_operations *old_fops;
 
 	if(minor>=VIDEO_NUM_DEVICES)
 		return -ENODEV;
Index: linux-2.6.15/drivers/message/i2o/i2o_proc.c
===================================================================
--- linux-2.6.15.orig/drivers/message/i2o/i2o_proc.c
+++ linux-2.6.15/drivers/message/i2o/i2o_proc.c
@@ -56,7 +56,7 @@
 typedef struct _i2o_proc_entry_t {
 	char *name;		/* entry name */
 	mode_t mode;		/* mode */
-	struct file_operations *fops;	/* open function */
+	const struct file_operations *fops;	/* open function */
 } i2o_proc_entry;
 
 /* global I2O /proc/i2o entry */
Index: linux-2.6.15/drivers/oprofile/oprofilefs.c
===================================================================
--- linux-2.6.15.orig/drivers/oprofile/oprofilefs.c
+++ linux-2.6.15/drivers/oprofile/oprofilefs.c
@@ -130,7 +130,7 @@ static struct file_operations ulong_ro_f
 
 
 static struct dentry * __oprofilefs_create_file(struct super_block * sb,
-	struct dentry * root, char const * name, struct file_operations * fops,
+	struct dentry * root, char const * name, const struct file_operations * fops,
 	int perm)
 {
 	struct dentry * dentry;
@@ -203,7 +203,7 @@ int oprofilefs_create_ro_atomic(struct s
 
  
 int oprofilefs_create_file(struct super_block * sb, struct dentry * root,
-	char const * name, struct file_operations * fops)
+	char const * name, const struct file_operations * fops)
 {
 	if (!__oprofilefs_create_file(sb, root, name, fops, 0644))
 		return -EFAULT;
@@ -212,7 +212,7 @@ int oprofilefs_create_file(struct super_
 
 
 int oprofilefs_create_file_perm(struct super_block * sb, struct dentry * root,
-	char const * name, struct file_operations * fops, int perm)
+	char const * name, const struct file_operations * fops, int perm)
 {
 	if (!__oprofilefs_create_file(sb, root, name, fops, perm))
 		return -EFAULT;
Index: linux-2.6.15/drivers/usb/core/file.c
===================================================================
--- linux-2.6.15.orig/drivers/usb/core/file.c
+++ linux-2.6.15/drivers/usb/core/file.c
@@ -24,15 +24,15 @@
 #include "usb.h"
 
 #define MAX_USB_MINORS	256
-static struct file_operations *usb_minors[MAX_USB_MINORS];
+static const struct file_operations *usb_minors[MAX_USB_MINORS];
 static DEFINE_SPINLOCK(minor_lock);
 
 static int usb_open(struct inode * inode, struct file * file)
 {
 	int minor = iminor(inode);
-	struct file_operations *c;
+	const struct file_operations *c;
 	int err = -ENODEV;
-	struct file_operations *old_fops, *new_fops = NULL;
+	const struct file_operations *old_fops, *new_fops = NULL;
 
 	spin_lock (&minor_lock);
 	c = usb_minors[minor];
Index: linux-2.6.15/fs/char_dev.c
===================================================================
--- linux-2.6.15.orig/fs/char_dev.c
+++ linux-2.6.15/fs/char_dev.c
@@ -201,7 +201,7 @@ int alloc_chrdev_region(dev_t *dev, unsi
 }
 
 int register_chrdev(unsigned int major, const char *name,
-		    struct file_operations *fops)
+		    const struct file_operations *fops)
 {
 	struct char_device_struct *cd;
 	struct cdev *cdev;
@@ -425,7 +425,7 @@ struct cdev *cdev_alloc(void)
 	return p;
 }
 
-void cdev_init(struct cdev *cdev, struct file_operations *fops)
+void cdev_init(struct cdev *cdev, const struct file_operations *fops)
 {
 	memset(cdev, 0, sizeof *cdev);
 	INIT_LIST_HEAD(&cdev->list);
Index: linux-2.6.15/fs/debugfs/inode.c
===================================================================
--- linux-2.6.15.orig/fs/debugfs/inode.c
+++ linux-2.6.15/fs/debugfs/inode.c
@@ -191,7 +191,7 @@ static int debugfs_create_by_name(const 
  */
 struct dentry *debugfs_create_file(const char *name, mode_t mode,
 				   struct dentry *parent, void *data,
-				   struct file_operations *fops)
+				   const struct file_operations *fops)
 {
 	struct dentry *dentry = NULL;
 	int error;
Index: linux-2.6.15/fs/inode.c
===================================================================
--- linux-2.6.15.orig/fs/inode.c
+++ linux-2.6.15/fs/inode.c
@@ -103,7 +103,7 @@ static struct inode *alloc_inode(struct 
 {
 	static struct address_space_operations empty_aops;
 	static struct inode_operations empty_iops;
-	static struct file_operations empty_fops;
+	static const struct file_operations empty_fops;
 	struct inode *inode;
 
 	if (sb->s_op->alloc_inode)
Index: linux-2.6.15/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.15.orig/fs/nfsd/vfs.c
+++ linux-2.6.15/fs/nfsd/vfs.c
@@ -718,7 +718,7 @@ nfsd_close(struct file *filp)
  * after it.
  */
 static inline void nfsd_dosync(struct file *filp, struct dentry *dp,
-			       struct file_operations *fop)
+			       const struct file_operations *fop)
 {
 	struct inode *inode = dp->d_inode;
 	int (*fsync) (struct file *, struct dentry *, int);
Index: linux-2.6.15/fs/proc/generic.c
===================================================================
--- linux-2.6.15.orig/fs/proc/generic.c
+++ linux-2.6.15/fs/proc/generic.c
@@ -535,7 +535,7 @@ static void proc_kill_inodes(struct proc
 		struct file * filp = list_entry(p, struct file, f_u.fu_list);
 		struct dentry * dentry = filp->f_dentry;
 		struct inode * inode;
-		struct file_operations *fops;
+		const struct file_operations *fops;
 
 		if (dentry->d_op != &proc_dentry_operations)
 			continue;
Index: linux-2.6.15/fs/proc/internal.h
===================================================================
--- linux-2.6.15.orig/fs/proc/internal.h
+++ linux-2.6.15/fs/proc/internal.h
@@ -30,7 +30,7 @@ do {						\
 
 #endif
 
-extern void create_seq_entry(char *name, mode_t mode, struct file_operations *f);
+extern void create_seq_entry(char *name, mode_t mode, const struct file_operations *f);
 extern int proc_exe_link(struct inode *, struct dentry **, struct vfsmount **);
 extern int proc_tid_stat(struct task_struct *,  char *);
 extern int proc_tgid_stat(struct task_struct *, char *);
Index: linux-2.6.15/fs/proc/proc_misc.c
===================================================================
--- linux-2.6.15.orig/fs/proc/proc_misc.c
+++ linux-2.6.15/fs/proc/proc_misc.c
@@ -555,7 +555,7 @@ static struct file_operations proc_sysrq
 
 struct proc_dir_entry *proc_root_kcore;
 
-void create_seq_entry(char *name, mode_t mode, struct file_operations *f)
+void create_seq_entry(char *name, mode_t mode, const struct file_operations *f)
 {
 	struct proc_dir_entry *entry;
 	entry = create_proc_entry(name, mode, NULL);
Index: linux-2.6.15/fs/select.c
===================================================================
--- linux-2.6.15.orig/fs/select.c
+++ linux-2.6.15/fs/select.c
@@ -210,7 +210,7 @@ int do_select(int n, fd_set_bits *fds, l
 		for (i = 0; i < n; ++rinp, ++routp, ++rexp) {
 			unsigned long in, out, ex, all_bits, bit = 1, mask, j;
 			unsigned long res_in = 0, res_out = 0, res_ex = 0;
-			struct file_operations *f_op = NULL;
+			const struct file_operations *f_op = NULL;
 			struct file *file = NULL;
 
 			in = *inp++; out = *outp++; ex = *exp++;
Index: linux-2.6.15/include/linux/cdev.h
===================================================================
--- linux-2.6.15.orig/include/linux/cdev.h
+++ linux-2.6.15/include/linux/cdev.h
@@ -5,13 +5,13 @@
 struct cdev {
 	struct kobject kobj;
 	struct module *owner;
-	struct file_operations *ops;
+	const struct file_operations *ops;
 	struct list_head list;
 	dev_t dev;
 	unsigned int count;
 };
 
-void cdev_init(struct cdev *, struct file_operations *);
+void cdev_init(struct cdev *, const struct file_operations *);
 
 struct cdev *cdev_alloc(void);
 
Index: linux-2.6.15/include/linux/debugfs.h
===================================================================
--- linux-2.6.15.orig/include/linux/debugfs.h
+++ linux-2.6.15/include/linux/debugfs.h
@@ -24,7 +24,7 @@ struct file_operations;
 #if defined(CONFIG_DEBUG_FS)
 struct dentry *debugfs_create_file(const char *name, mode_t mode,
 				   struct dentry *parent, void *data,
-				   struct file_operations *fops);
+				   const struct file_operations *fops);
 
 struct dentry *debugfs_create_dir(const char *name, struct dentry *parent);
 
Index: linux-2.6.15/include/linux/fs.h
===================================================================
--- linux-2.6.15.orig/include/linux/fs.h
+++ linux-2.6.15/include/linux/fs.h
@@ -456,7 +456,7 @@ struct inode {
 	struct semaphore	i_sem;
 	struct rw_semaphore	i_alloc_sem;
 	struct inode_operations	*i_op;
-	struct file_operations	*i_fop;	/* former ->i_op->default_file_ops */
+	const struct file_operations	*i_fop;	/* former ->i_op->default_file_ops */
 	struct super_block	*i_sb;
 	struct file_lock	*i_flock;
 	struct address_space	*i_mapping;
@@ -596,7 +596,7 @@ struct file {
 	} f_u;
 	struct dentry		*f_dentry;
 	struct vfsmount         *f_vfsmnt;
-	struct file_operations	*f_op;
+	const struct file_operations	*f_op;
 	atomic_t		f_count;
 	unsigned int 		f_flags;
 	mode_t			f_mode;
@@ -1356,7 +1356,7 @@ extern void bd_release(struct block_devi
 extern int alloc_chrdev_region(dev_t *, unsigned, unsigned, const char *);
 extern int register_chrdev_region(dev_t, unsigned, const char *);
 extern int register_chrdev(unsigned int, const char *,
-			   struct file_operations *);
+			   const struct file_operations *);
 extern int unregister_chrdev(unsigned int, const char *);
 extern void unregister_chrdev_region(dev_t, unsigned);
 extern int chrdev_open(struct inode *, struct file *);
Index: linux-2.6.15/include/linux/input.h
===================================================================
--- linux-2.6.15.orig/include/linux/input.h
+++ linux-2.6.15/include/linux/input.h
@@ -954,7 +954,7 @@ struct input_handler {
 	struct input_handle* (*connect)(struct input_handler *handler, struct input_dev *dev, struct input_device_id *id);
 	void (*disconnect)(struct input_handle *handle);
 
-	struct file_operations *fops;
+	const struct file_operations *fops;
 	int minor;
 	char *name;
 
Index: linux-2.6.15/include/linux/miscdevice.h
===================================================================
--- linux-2.6.15.orig/include/linux/miscdevice.h
+++ linux-2.6.15/include/linux/miscdevice.h
@@ -36,7 +36,7 @@ struct class_device;
 struct miscdevice  {
 	int minor;
 	const char *name;
-	struct file_operations *fops;
+	const struct file_operations *fops;
 	struct list_head list;
 	struct device *dev;
 	struct class_device *class;
Index: linux-2.6.15/include/linux/oprofile.h
===================================================================
--- linux-2.6.15.orig/include/linux/oprofile.h
+++ linux-2.6.15/include/linux/oprofile.h
@@ -74,10 +74,10 @@ void oprofile_add_trace(unsigned long ei
  * the specified file operations.
  */
 int oprofilefs_create_file(struct super_block * sb, struct dentry * root,
-	char const * name, struct file_operations * fops);
+	char const * name, const struct file_operations * fops);
 
 int oprofilefs_create_file_perm(struct super_block * sb, struct dentry * root,
-	char const * name, struct file_operations * fops, int perm);
+	char const * name, const struct file_operations * fops, int perm);
  
 /** Create a file for read/write access to an unsigned long. */
 int oprofilefs_create_ulong(struct super_block * sb, struct dentry * root,
Index: linux-2.6.15/include/linux/proc_fs.h
===================================================================
--- linux-2.6.15.orig/include/linux/proc_fs.h
+++ linux-2.6.15/include/linux/proc_fs.h
@@ -57,7 +57,7 @@ struct proc_dir_entry {
 	gid_t gid;
 	unsigned long size;
 	struct inode_operations * proc_iops;
-	struct file_operations * proc_fops;
+	const struct file_operations * proc_fops;
 	get_info_t *get_info;
 	struct module *owner;
 	struct proc_dir_entry *next, *parent, *subdir;
@@ -181,7 +181,7 @@ static inline struct proc_dir_entry *pro
 }
 
 static inline struct proc_dir_entry *proc_net_fops_create(const char *name,
-	mode_t mode, struct file_operations *fops)
+	mode_t mode, const struct file_operations *fops)
 {
 	struct proc_dir_entry *res = create_proc_entry(name, mode, proc_net);
 	if (res)
Index: linux-2.6.15/include/linux/sound.h
===================================================================
--- linux-2.6.15.orig/include/linux/sound.h
+++ linux-2.6.15/include/linux/sound.h
@@ -30,12 +30,12 @@
  */
  
 struct device;
-extern int register_sound_special(struct file_operations *fops, int unit);
-extern int register_sound_special_device(struct file_operations *fops, int unit, struct device *dev);
-extern int register_sound_mixer(struct file_operations *fops, int dev);
-extern int register_sound_midi(struct file_operations *fops, int dev);
-extern int register_sound_dsp(struct file_operations *fops, int dev);
-extern int register_sound_synth(struct file_operations *fops, int dev);
+extern int register_sound_special(const struct file_operations *fops, int unit);
+extern int register_sound_special_device(const struct file_operations *fops, int unit, struct device *dev);
+extern int register_sound_mixer(const struct file_operations *fops, int dev);
+extern int register_sound_midi(const struct file_operations *fops, int dev);
+extern int register_sound_dsp(const struct file_operations *fops, int dev);
+extern int register_sound_synth(const struct file_operations *fops, int dev);
 
 extern void unregister_sound_special(int unit);
 extern void unregister_sound_mixer(int unit);
Index: linux-2.6.15/include/linux/sunrpc/stats.h
===================================================================
--- linux-2.6.15.orig/include/linux/sunrpc/stats.h
+++ linux-2.6.15/include/linux/sunrpc/stats.h
@@ -50,7 +50,7 @@ struct proc_dir_entry *	rpc_proc_registe
 void			rpc_proc_unregister(const char *);
 void			rpc_proc_zero(struct rpc_program *);
 struct proc_dir_entry *	svc_proc_register(struct svc_stat *,
-					  struct file_operations *);
+					  const struct file_operations *);
 void			svc_proc_unregister(const char *);
 
 void			svc_seq_show(struct seq_file *,
@@ -65,7 +65,7 @@ static inline void rpc_proc_unregister(c
 static inline void rpc_proc_zero(struct rpc_program *p) {}
 
 static inline struct proc_dir_entry *svc_proc_register(struct svc_stat *s,
-						       struct file_operations *f) { return NULL; }
+						       const struct file_operations *f) { return NULL; }
 static inline void svc_proc_unregister(const char *p) {}
 
 static inline void svc_seq_show(struct seq_file *seq,
Index: linux-2.6.15/include/linux/usb.h
===================================================================
--- linux-2.6.15.orig/include/linux/usb.h
+++ linux-2.6.15/include/linux/usb.h
@@ -606,7 +606,7 @@ extern struct bus_type usb_bus_type;
  */
 struct usb_class_driver {
 	char *name;
-	struct file_operations *fops;
+	const struct file_operations *fops;
 	int minor_base;
 };
 
Index: linux-2.6.15/include/linux/videodev2.h
===================================================================
--- linux-2.6.15.orig/include/linux/videodev2.h
+++ linux-2.6.15/include/linux/videodev2.h
@@ -64,7 +64,7 @@ struct video_device
 	int minor;
 
 	/* device ops + callbacks */
-	struct file_operations *fops;
+	const struct file_operations *fops;
 	void (*release)(struct video_device *vfd);
 
 
Index: linux-2.6.15/include/sound/core.h
===================================================================
--- linux-2.6.15.orig/include/sound/core.h
+++ linux-2.6.15/include/sound/core.h
@@ -246,7 +246,7 @@ struct _snd_minor {
 	int number;			/* minor number */
 	int device;			/* device number */
 	const char *comment;		/* for /proc/asound/devices */
-	struct file_operations *f_ops;	/* file operations */
+	const struct file_operations *f_ops;	/* file operations */
 	char name[0];			/* device name (keep at the end of
 								structure) */
 };
Index: linux-2.6.15/net/sunrpc/rpc_pipe.c
===================================================================
--- linux-2.6.15.orig/net/sunrpc/rpc_pipe.c
+++ linux-2.6.15/net/sunrpc/rpc_pipe.c
@@ -371,7 +371,7 @@ enum {
  */
 struct rpc_filelist {
 	char *name;
-	struct file_operations *i_fop;
+	const struct file_operations *i_fop;
 	int mode;
 };
 
Index: linux-2.6.15/net/sunrpc/stats.c
===================================================================
--- linux-2.6.15.orig/net/sunrpc/stats.c
+++ linux-2.6.15/net/sunrpc/stats.c
@@ -110,7 +110,7 @@ void svc_seq_show(struct seq_file *seq, 
  * Register/unregister RPC proc files
  */
 static inline struct proc_dir_entry *
-do_register(const char *name, void *data, struct file_operations *fops)
+do_register(const char *name, void *data, const struct file_operations *fops)
 {
 	struct proc_dir_entry *ent;
 
@@ -138,7 +138,7 @@ rpc_proc_unregister(const char *name)
 }
 
 struct proc_dir_entry *
-svc_proc_register(struct svc_stat *statp, struct file_operations *fops)
+svc_proc_register(struct svc_stat *statp, const struct file_operations *fops)
 {
 	return do_register(statp->program->pg_name, statp, fops);
 }
Index: linux-2.6.15/sound/core/init.c
===================================================================
--- linux-2.6.15.orig/sound/core/init.c
+++ linux-2.6.15/sound/core/init.c
@@ -163,7 +163,8 @@ int snd_card_disconnect(snd_card_t * car
 	struct snd_monitor_file *mfile;
 	struct file *file;
 	struct snd_shutdown_f_ops *s_f_ops;
-	struct file_operations *f_ops, *old_f_ops;
+	struct file_operations *f_ops;
+	const struct file_operations *old_f_ops;
 	int err;
 
 	spin_lock(&card->files_lock);
Index: linux-2.6.15/sound/core/sound.c
===================================================================
--- linux-2.6.15.orig/sound/core/sound.c
+++ linux-2.6.15/sound/core/sound.c
@@ -127,7 +127,7 @@ static int snd_open(struct inode *inode,
 	int card = SNDRV_MINOR_CARD(minor);
 	int dev = SNDRV_MINOR_DEVICE(minor);
 	snd_minor_t *mptr = NULL;
-	struct file_operations *old_fops;
+	const struct file_operations *old_fops;
 	int err = 0;
 
 	if (dev != SNDRV_MINOR_GLOBAL) {
Index: linux-2.6.15/sound/sound_core.c
===================================================================
--- linux-2.6.15.orig/sound/sound_core.c
+++ linux-2.6.15/sound/sound_core.c
@@ -53,7 +53,7 @@
 struct sound_unit
 {
 	int unit_minor;
-	struct file_operations *unit_fops;
+	const struct file_operations *unit_fops;
 	struct sound_unit *next;
 	char name[32];
 };
@@ -73,7 +73,7 @@ EXPORT_SYMBOL(sound_class);
  *	join into it. Called with the lock asserted
  */
 
-static int __sound_insert_unit(struct sound_unit * s, struct sound_unit **list, struct file_operations *fops, int index, int low, int top)
+static int __sound_insert_unit(struct sound_unit * s, struct sound_unit **list, const struct file_operations *fops, int index, int low, int top)
 {
 	int n=low;
 
@@ -153,7 +153,7 @@ static DEFINE_SPINLOCK(sound_loader_lock
  *	list. Acquires locks as needed
  */
 
-static int sound_insert_unit(struct sound_unit **list, struct file_operations *fops, int index, int low, int top, const char *name, umode_t mode, struct device *dev)
+static int sound_insert_unit(struct sound_unit **list, const struct file_operations *fops, int index, int low, int top, const char *name, umode_t mode, struct device *dev)
 {
 	struct sound_unit *s = kmalloc(sizeof(*s), GFP_KERNEL);
 	int r;
@@ -237,7 +237,7 @@ static struct sound_unit *chains[SOUND_S
  *	a negative error code is returned.
  */
  
-int register_sound_special_device(struct file_operations *fops, int unit,
+int register_sound_special_device(const struct file_operations *fops, int unit,
 				  struct device *dev)
 {
 	const int chain = unit % SOUND_STEP;
@@ -301,7 +301,7 @@ int register_sound_special_device(struct
  
 EXPORT_SYMBOL(register_sound_special_device);
 
-int register_sound_special(struct file_operations *fops, int unit)
+int register_sound_special(const struct file_operations *fops, int unit)
 {
 	return register_sound_special_device(fops, unit, NULL);
 }
@@ -318,7 +318,7 @@ EXPORT_SYMBOL(register_sound_special);
  *	number is returned, on failure a negative error code is returned.
  */
 
-int register_sound_mixer(struct file_operations *fops, int dev)
+int register_sound_mixer(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[0], fops, dev, 0, 128,
 				 "mixer", S_IRUSR | S_IWUSR, NULL);
@@ -336,7 +336,7 @@ EXPORT_SYMBOL(register_sound_mixer);
  *	number is returned, on failure a negative error code is returned.
  */
 
-int register_sound_midi(struct file_operations *fops, int dev)
+int register_sound_midi(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[2], fops, dev, 2, 130,
 				 "midi", S_IRUSR | S_IWUSR, NULL);
@@ -362,7 +362,7 @@ EXPORT_SYMBOL(register_sound_midi);
  *	and will always allocate them as a matching pair - eg dsp3/audio3
  */
 
-int register_sound_dsp(struct file_operations *fops, int dev)
+int register_sound_dsp(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[3], fops, dev, 3, 131,
 				 "dsp", S_IWUSR | S_IRUSR, NULL);
@@ -381,7 +381,7 @@ EXPORT_SYMBOL(register_sound_dsp);
  */
 
 
-int register_sound_synth(struct file_operations *fops, int dev)
+int register_sound_synth(const struct file_operations *fops, int dev)
 {
 	return sound_insert_unit(&chains[9], fops, dev, 9, 137,
 				 "synth", S_IRUSR | S_IWUSR, NULL);
@@ -501,7 +501,7 @@ int soundcore_open(struct inode *inode, 
 	int chain;
 	int unit = iminor(inode);
 	struct sound_unit *s;
-	struct file_operations *new_fops = NULL;
+	const struct file_operations *new_fops = NULL;
 
 	chain=unit&0x0F;
 	if(chain==4 || chain==5)	/* dsp/audio/dsp16 */
@@ -540,7 +540,7 @@ int soundcore_open(struct inode *inode, 
 		 * switching ->f_op in the first place.
 		 */
 		int err = 0;
-		struct file_operations *old_fops = file->f_op;
+		const struct file_operations *old_fops = file->f_op;
 		file->f_op = new_fops;
 		spin_unlock(&sound_loader_lock);
 		if(file->f_op->open)
Index: linux-2.6.15/drivers/telephony/phonedev.c
===================================================================
--- linux-2.6.15.orig/drivers/telephony/phonedev.c
+++ linux-2.6.15/drivers/telephony/phonedev.c
@@ -48,7 +48,7 @@ static int phone_open(struct inode *inod
 	unsigned int minor = iminor(inode);
 	int err = 0;
 	struct phone_device *p;
-	struct file_operations *old_fops, *new_fops = NULL;
+	const struct file_operations *old_fops, *new_fops = NULL;
 
 	if (minor >= PHONE_NUM_DEVICES)
 		return -ENODEV;



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
                   ` (3 preceding siblings ...)
  2006-01-06 21:51 ` [patch 4/4] Actually make the f_ops field const Arjan van de Ven
@ 2006-01-06 21:55 ` Arjan van de Ven
  2006-01-06 22:46   ` Eric Dumazet
  4 siblings, 1 reply; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-06 21:55 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm

On Fri, 2006-01-06 at 22:45 +0100, Arjan van de Ven wrote:
> Hi,
> 
> this series allows drivers to have "const" file_operations, by making
> the f_ops field in the inode const. This has another benefit, there have

ok there was a sentence missing here. The first benefit is that this
moves these hot datastructures to the rodata section, which means they
won't accidentally be doing false cacheline sharing.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-06 21:55 ` [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
@ 2006-01-06 22:46   ` Eric Dumazet
  2006-01-07  0:29     ` Andrew Morton
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Dumazet @ 2006-01-06 22:46 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, akpm

Arjan van de Ven a écrit :
> On Fri, 2006-01-06 at 22:45 +0100, Arjan van de Ven wrote:
>> Hi,
>>
>> this series allows drivers to have "const" file_operations, by making
>> the f_ops field in the inode const. This has another benefit, there have
> 
> ok there was a sentence missing here. The first benefit is that this
> moves these hot datastructures to the rodata section, which means they
> won't accidentally be doing false cacheline sharing.
> 

Definitly a good thing I agree.

But your patches miss to really declare all 'struct file_operations' as const, 
dont they ?


On my x86_64 machine, I managed to reduce by 10% .data section by moving all 
file_operations, but also 'address_space_operations', 'inode_operations, 
super_operations, dentry_operations, seq_operations, ... to rodata section.

size vmlinux*
    text    data     bss     dec     hex filename
2476156  522236  244868 3243260  317cfc vmlinux
2588685  571348  246692 3406725  33fb85 vmlinux.old

Eric


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 2/4] fix input layer f_ops abuse
  2006-01-06 21:47 ` [patch 2/4] fix input layer f_ops abuse Arjan van de Ven
@ 2006-01-06 23:12   ` Dmitry Torokhov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2006-01-06 23:12 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, akpm

On 1/6/06, Arjan van de Ven <arjan@infradead.org> wrote:
> From: Arjan van de Ven <arjan@infradead.org>
>
> The input layer has an assignment to a live ->fops, just after creating the
> fops as a duplicate of another one. Just move this assignment a few lines up to avoid
> the race and the assignment to a live fops
>

I do not understand how it will fix the "race", there is still a split
second between "entry" having default fops and modified one. I'd
prefer you fix the comment to reflect that the only change is because
fops is now constant.

--
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-06 22:46   ` Eric Dumazet
@ 2006-01-07  0:29     ` Andrew Morton
  2006-01-07  1:01       ` Arnd Bergmann
                         ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Andrew Morton @ 2006-01-07  0:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: arjan, linux-kernel

Eric Dumazet <dada1@cosmosbay.com> wrote:
>
> Arjan van de Ven a écrit :
> > On Fri, 2006-01-06 at 22:45 +0100, Arjan van de Ven wrote:
> >> Hi,
> >>
> >> this series allows drivers to have "const" file_operations, by making
> >> the f_ops field in the inode const. This has another benefit, there have
> > 
> > ok there was a sentence missing here. The first benefit is that this
> > moves these hot datastructures to the rodata section, which means they
> > won't accidentally be doing false cacheline sharing.
> > 
> 
> Definitly a good thing I agree.
> 
> But your patches miss to really declare all 'struct file_operations' as const, 
> dont they ?
> 
> 
> On my x86_64 machine, I managed to reduce by 10% .data section by moving all 
> file_operations, but also 'address_space_operations', 'inode_operations, 
> super_operations, dentry_operations, seq_operations, ... to rodata section.
> 
> size vmlinux*
>     text    data     bss     dec     hex filename
> 2476156  522236  244868 3243260  317cfc vmlinux
> 2588685  571348  246692 3406725  33fb85 vmlinux.old
> 

Confused.   Why should this result in an aggregate reduction in vmlinux size?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-07  0:29     ` Andrew Morton
@ 2006-01-07  1:01       ` Arnd Bergmann
  2006-01-07  7:40       ` Eric Dumazet
  2006-01-07  8:33       ` Arjan van de Ven
  2 siblings, 0 replies; 14+ messages in thread
From: Arnd Bergmann @ 2006-01-07  1:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Eric Dumazet, arjan, linux-kernel

Am Samstag, 7. Januar 2006 01:29 schrieb Andrew Morton:
> > size vmlinux*
> >     text    data     bss     dec     hex filename
> > 2476156  522236  244868 3243260  317cfc vmlinux
> > 2588685  571348  246692 3406725  33fb85 vmlinux.old
>
> Confused.   Why should this result in an aggregate reduction in vmlinux
> size?

The size tool only displays the sum of .text, .data and .bss, but completely 
ignores other sections. So if you manage to move part of the object e.g.
into .rodata or .initdata, it will show a smaller size although the size of 
the actual vmlinux file stays the same.

Erics point was just about .data getting smaller, not about the bogus sum.

	Arnd <><

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-07  0:29     ` Andrew Morton
  2006-01-07  1:01       ` Arnd Bergmann
@ 2006-01-07  7:40       ` Eric Dumazet
  2006-01-07  8:33       ` Arjan van de Ven
  2 siblings, 0 replies; 14+ messages in thread
From: Eric Dumazet @ 2006-01-07  7:40 UTC (permalink / raw)
  To: Andrew Morton; +Cc: arjan, linux-kernel

Andrew Morton a écrit :
> Eric Dumazet <dada1@cosmosbay.com> wrote:
>> On my x86_64 machine, I managed to reduce by 10% .data section by moving all 
>> file_operations, but also 'address_space_operations', 'inode_operations, 
>> super_operations, dentry_operations, seq_operations, ... to rodata section.
>>
>> size vmlinux*
>>     text    data     bss     dec     hex filename
>> 2476156  522236  244868 3243260  317cfc vmlinux
>> 2588685  571348  246692 3406725  33fb85 vmlinux.old
>>
> 
> Confused.   Why should this result in an aggregate reduction in vmlinux size?
> 
> 

Sorry, vmlinux.old was with CONFIG_FRAME_POINTER and CONFIG_MODULES, not 'vmlinux'

If I add back CONFIG_FRAME_POINTER / CONFIG_MODULES on my config I get :

# size vmlinux
    text    data     bss     dec     hex filename
2627717  523988  244868 3396573  33d3dd vmlinux


Aggregate reduction might be possible because of 'const' : Compiler (gcc-3.4.4 
here) is free to make further optimizations ?

# size -A vmlinux
vmlinux  :
section                      size                   addr
.text                     2128492   18446744071563116544
__ex_table                  15408   18446744071565245040
.rodata                    256486   18446744071565260448
.pci_fixup                   2112   18446744071565516944
__ksymtab                   34336   18446744071565519056
__ksymtab_gpl                5488   18446744071565553392
__kcrctab                       0   18446744071565558880
__kcrctab_gpl                   0   18446744071565558880
__ksymtab_strings           54978   18446744071565558880
__param                      2560   18446744071565613864
.data                      408392   18446744071565616448
.bss                       244868   18446744071566024896
.data.cacheline_aligned     23936   18446744071566270464
.data.read_mostly            7296   18446744071566294400
.vsyscall_0                   319   18446744073699065856
.xtime_lock                     8   18446744073699066176
.vxtime                        48   18446744073699066192
.wall_jiffies                   8   18446744073699066240
.sys_tz                         8   18446744073699066256
.sysctl_vsyscall                4   18446744073699066272
.xtime                         16   18446744073699066288
.jiffies                        8   18446744073699066304
.vsyscall_1                    47   18446744073699066880
.vsyscall_2                    13   18446744073699067904
.vsyscall_3                    13   18446744073699068928
.data.init_task              8192   18446744071566311424
.init.text                 124192   18446744071566319616
.init.data                  42584   18446744071566443808
.init.setup                  2184   18446744071566486400
.initcall.init               1240   18446744071566488584
.con_initcall.init             24   18446744071566489824
.security_initcall.init         0   18446744071566489848
.altinstructions              139   18446744071566489848
.altinstr_replacement          93   18446744071566489987
.exit.text                   2907   18446744071566490080
.init.ramfs                   134   18446744071566495744
.data.percpu                30040   18446744071566495936
.comment                    11808                      0
.note.GNU-stack                 0                      0
Total                     3408381


# size -A vmlinux.old
vmlinux.4  :
section                      size                   addr
.text                     2131420   18446744071563116544
__ex_table                  15408   18446744071565247968
.rodata                    214318   18446744071565263392
.pci_fixup                   2112   18446744071565477712
__ksymtab                   34368   18446744071565479824
__ksymtab_gpl                5488   18446744071565514192
__kcrctab                       0   18446744071565519680
__kcrctab_gpl                   0   18446744071565519680
__ksymtab_strings           55010   18446744071565519680
__param                      2560   18446744071565574696
.data                      451848   18446744071565577280
.bss                       246692   18446744071566029184
.data.cacheline_aligned     23744   18446744071566278656
.data.read_mostly           11328   18446744071566302400
.vsyscall_0                   319   18446744073699065856
.xtime_lock                     8   18446744073699066176
.vxtime                        48   18446744073699066192
.wall_jiffies                   8   18446744073699066240
.sys_tz                         8   18446744073699066256
.sysctl_vsyscall                4   18446744073699066272
.xtime                         16   18446744073699066288
.jiffies                        8   18446744073699066304
.vsyscall_1                    47   18446744073699066880
.vsyscall_2                    13   18446744073699067904
.vsyscall_3                    13   18446744073699068928
.data.init_task              8192   18446744071566319616
.init.text                 124352   18446744071566327808
.init.data                  42616   18446744071566452160
.init.setup                  2208   18446744071566494784
.initcall.init               1248   18446744071566496992
.con_initcall.init             24   18446744071566498240
.security_initcall.init         0   18446744071566498264
.altinstructions              139   18446744071566498264
.altinstr_replacement          93   18446744071566498403
.exit.text                   2891   18446744071566498496
.init.ramfs                   134   18446744071566503936
.data.percpu                30040   18446744071566504128
.comment                    11844                      0
.note.GNU-stack                 0                      0
Total                     3418569



Eric


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 4/4] Actually make the f_ops field const
  2006-01-06 21:51 ` [patch 4/4] Actually make the f_ops field const Arjan van de Ven
@ 2006-01-07  8:00   ` Andrew Morton
  2006-01-07  8:13     ` Arjan van de Ven
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2006-01-07  8:00 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: linux-kernel, aviro

Arjan van de Ven <arjan@infradead.org> wrote:
>
> Mark the f_ops members of inodes as const, as well as fix the ripple-through
>  this causes by places that copy this f_ops and then "do stuff" with it.
>  (some of the "do stuff" is quite unpleasant..)

Right now is the worst possible time in the kernel cycle to be raising
large many-file patches like this.  You need to either take a look at
what's coming in -mm or wait until all the git trees have merged up then do
the patch against -linus.

In this case there are mulitple new references to non-const file_operations
added to Linus's tree by alsa 48 hours ago.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 4/4] Actually make the f_ops field const
  2006-01-07  8:00   ` Andrew Morton
@ 2006-01-07  8:13     ` Arjan van de Ven
  0 siblings, 0 replies; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-07  8:13 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, aviro

On Sat, 2006-01-07 at 00:00 -0800, Andrew Morton wrote:
> Arjan van de Ven <arjan@infradead.org> wrote:
> >
> > Mark the f_ops members of inodes as const, as well as fix the ripple-through
> >  this causes by places that copy this f_ops and then "do stuff" with it.
> >  (some of the "do stuff" is quite unpleasant..)
> 
> Right now is the worst possible time in the kernel cycle to be raising
> large many-file patches like this.  You need to either take a look at
> what's coming in -mm or wait until all the git trees have merged up then do
> the patch against -linus.
> 
> In this case there are mulitple new references to non-const file_operations
> added to Linus's tree by alsa 48 hours ago.

argh things move really fast; I'll redo it immediately


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [patch 0/4] Series to allow a "const" file_operations struct
  2006-01-07  0:29     ` Andrew Morton
  2006-01-07  1:01       ` Arnd Bergmann
  2006-01-07  7:40       ` Eric Dumazet
@ 2006-01-07  8:33       ` Arjan van de Ven
  2 siblings, 0 replies; 14+ messages in thread
From: Arjan van de Ven @ 2006-01-07  8:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Eric Dumazet, linux-kernel


> 
> Confused.   Why should this result in an aggregate reduction in vmlinux size?

there shouldn't be. It's just moving these things from the .data section
(where cachelines get dirtied all the time) to .rodata (where they're
not)


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2006-01-07  8:33 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-06 21:45 [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
2006-01-06 21:46 ` [patch 1/4] fix some f_ops abuse in acpi Arjan van de Ven
2006-01-06 21:47 ` [patch 2/4] fix input layer f_ops abuse Arjan van de Ven
2006-01-06 23:12   ` Dmitry Torokhov
2006-01-06 21:49 ` [patch 3/4] fix cifs bugs wrt writing to f_ops Arjan van de Ven
2006-01-06 21:51 ` [patch 4/4] Actually make the f_ops field const Arjan van de Ven
2006-01-07  8:00   ` Andrew Morton
2006-01-07  8:13     ` Arjan van de Ven
2006-01-06 21:55 ` [patch 0/4] Series to allow a "const" file_operations struct Arjan van de Ven
2006-01-06 22:46   ` Eric Dumazet
2006-01-07  0:29     ` Andrew Morton
2006-01-07  1:01       ` Arnd Bergmann
2006-01-07  7:40       ` Eric Dumazet
2006-01-07  8:33       ` Arjan van de Ven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox