Fix proc_file_write missing ppos update

PATCH In review Stefani Seibold 6 participants 13 messages Aug 29, 2009 – Sep 12, 2009

Subsystems (24)

Documentation Arm (32-bit) Media (V4L/DVB) SCSI +20 more subsystems

lore

Share & tools

lore

Stefani Seibold

author

2009-08-29 16:38 UTC #0 lore raw

PATCH 1/1 Fix proc_file_write missing ppos update

The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.

This prevent the usage of multiple sequently writes on an opened proc file, because it is impossible to distinguish these due the offset is always 0.

The patch has no side effects, because the old implementaion has no ability to access the file offset pointer.

Out of tree should work, also if there not using the new prototype definition, because the new file offset pointer argument will be passed as last parameter.

The patch is against 2.6.31-rc8

Signed-off-by Stefani Seibold <[email protected]>

---
 fs/proc/generic.c                                      |    5 ++--
 include/linux/proc_fs.h                                |    2 -
 Documentation/DocBook/procfs-guide.tmpl                |    1 
 Documentation/DocBook/procfs_example.c                 |    3 +-
 arch/alpha/kernel/srm_env.c                            |    2 -
 arch/arm/mm/alignment.c                                |    2 -
 arch/blackfin/kernel/kgdb_test.c                       |    2 -
 arch/mips/lasat/picvue_proc.c                          |    4 +--
 arch/powerpc/platforms/iseries/mf.c                    |    6 ++---
 arch/um/drivers/mconsole_kern.c                        |    2 -
 arch/um/kernel/exitcode.c                              |    2 -
 arch/um/kernel/process.c                               |    3 +-
 drivers/acpi/debug.c                                   |    2 -
 drivers/block/DAC960.c                                 |    3 +-
 drivers/ide/ide-proc.c                                 |    6 +++--
 drivers/isdn/hardware/eicon/divasproc.c                |    8 +++----
 drivers/macintosh/via-pmu.c                            |    4 +--
 drivers/media/dvb/ttpci/av7110_ir.c                    |    2 -
 drivers/media/video/cpia.c                             |    2 -
 drivers/net/wireless/ipw2x00/libipw_module.c           |    2 -
 drivers/net/wireless/ray_cs.c                          |    4 +--
 drivers/parisc/led.c                                   |    2 -
 drivers/platform/x86/asus_acpi.c                       |   18 ++++++++---------
 drivers/platform/x86/thinkpad_acpi.c                   |    2 -
 drivers/platform/x86/toshiba_acpi.c                    |    2 -
 drivers/pnp/pnpbios/proc.c                             |    2 -
 drivers/s390/block/dasd_proc.c                         |    2 -
 drivers/s390/crypto/zcrypt_api.c                       |    2 -
 drivers/scsi/scsi_proc.c                               |    2 -
 drivers/staging/epl/proc_fs.c                          |    4 +--
 drivers/staging/rtl8192su/ieee80211/ieee80211_module.c |    2 -
 drivers/video/clps711xfb.c                             |    4 +--
 drivers/video/via/viafbdev.c                           |   15 +++++++++-----
 kernel/profile.c                                       |    2 -
 34 files changed, 69 insertions(+), 57 deletions(-)

include/linux/proc_fs.h +1 -1

diff -u -N -r write_proc.orig/include/linux/proc_fs.h write_proc/include/linux/proc_fs.h
@@ -46,7 +46,7 @@
 typedef	int (read_proc_t)(char *page, char **start, off_t off,
 			  int count, int *eof, void *data);
 typedef	int (write_proc_t)(struct file *file, const char __user *buffer,
-			   unsigned long count, void *data);
+			   unsigned long count, void *data, off_t off);
 
 struct proc_dir_entry {
 	unsigned int low_ino;

fs/proc/generic.c +3 -2

diff -u -N -r write_proc.orig/fs/proc/generic.c write_proc/fs/proc/generic.c
@@ -219,9 +219,10 @@
 		pde->pde_users++;
 		spin_unlock(&pde->pde_unload_lock);
 
-		/* FIXME: does this routine need ppos?  probably... */
-		rv = pde->write_proc(file, buffer, count, pde->data);
+		rv = pde->write_proc(file, buffer, count, pde->data, *ppos);
 		pde_users_dec(pde);
+		if (rv > 0)
+			*ppos += rv;
 	}
 	return rv;
 }

arch/alpha/kernel/srm_env.c +1 -1

diff -u -N -r write_proc.orig/arch/alpha/kernel/srm_env.c write_proc/arch/alpha/kernel/srm_env.c
@@ -106,7 +106,7 @@
 
 static int
 srm_env_write(struct file *file, const char __user *buffer, unsigned long count,
-		void *data)
+		void *data, off_t off)
 {
 	int res;
 	srm_env_t	*entry;

arch/arm/mm/alignment.c +1 -1

diff -u -N -r write_proc.orig/arch/arm/mm/alignment.c write_proc/arch/arm/mm/alignment.c
@@ -120,7 +120,7 @@
 }
 
 static int proc_alignment_write(struct file *file, const char __user *buffer,
-				unsigned long count, void *data)
+				unsigned long count, void *data, off_t off)
 {
 	char mode;

arch/blackfin/kernel/kgdb_test.c +1 -1

diff -u -N -r write_proc.orig/arch/blackfin/kernel/kgdb_test.c write_proc/arch/blackfin/kernel/kgdb_test.c
@@ -90,7 +90,7 @@
 }
 
 static int test_write_proc(struct file *file, const char *buffer,
-				 unsigned long count, void *data)
+				 unsigned long count, void *data, off_t off)
 {
 	if (count >= 256)
 		len = 255;

arch/mips/lasat/picvue_proc.c +2 -2

diff -u -N -r write_proc.orig/arch/mips/lasat/picvue_proc.c write_proc/arch/mips/lasat/picvue_proc.c
@@ -58,7 +58,7 @@
 }
 
 static int pvc_proc_write_line(struct file *file, const char *buffer,
-			   unsigned long count, void *data)
+			   unsigned long count, void *data, off_t off)
 {
 	int origcount = count;
 	int lineno = *(int *)data;

@@ -86,7 +86,7 @@
 }
 
 static int pvc_proc_write_scroll(struct file *file, const char *buffer,
-			   unsigned long count, void *data)
+			   unsigned long count, void *data, off_t off)
 {
 	int origcount = count;
 	int cmd = simple_strtol(buffer, NULL, 10);

arch/powerpc/platforms/iseries/mf.c +3 -3

diff -u -N -r write_proc.orig/arch/powerpc/platforms/iseries/mf.c write_proc/arch/powerpc/platforms/iseries/mf.c
@@ -1003,7 +1003,7 @@
 }
 
 static int proc_mf_change_side(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	char side;
 	u64 newSide;

@@ -1110,7 +1110,7 @@
 }
 
 static int proc_mf_change_src(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	char stkbuf[10];

@@ -1136,7 +1136,7 @@
 }
 
 static int proc_mf_change_cmdline(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	struct vsp_cmd_data vsp_cmd;
 	dma_addr_t dma_addr;

arch/um/drivers/mconsole_kern.c +1 -1

diff -u -N -r write_proc.orig/arch/um/drivers/mconsole_kern.c write_proc/arch/um/drivers/mconsole_kern.c
@@ -834,7 +834,7 @@
 __initcall(mconsole_init);
 
 static int write_proc_mconsole(struct file *file, const char __user *buffer,
-			       unsigned long count, void *data)
+			       unsigned long count, void *data, off_t off)
 {
 	char *buf;

arch/um/kernel/exitcode.c +1 -1

diff -u -N -r write_proc.orig/arch/um/kernel/exitcode.c write_proc/arch/um/kernel/exitcode.c
@@ -39,7 +39,7 @@
 }
 
 static int write_proc_exitcode(struct file *file, const char __user *buffer,
-			       unsigned long count, void *data)
+			       unsigned long count, void *data, off_t off)
 {
 	char *end, buf[sizeof("nnnnn\0")];
 	int tmp;

arch/um/kernel/process.c +2 -1

diff -u -N -r write_proc.orig/arch/um/kernel/process.c write_proc/arch/um/kernel/process.c
@@ -345,7 +345,8 @@
 	return strlen(buf);
 }
 
-static int proc_write_sysemu(struct file *file,const char __user *buf, unsigned long count,void *data)
+static int proc_write_sysemu(struct file *file, const char __user *buf,
+		unsigned long count, void *data, off_t off)
 {
 	char tmp[2];

Documentation/DocBook/procfs_example.c +2 -1

diff -u -N -r write_proc.orig/Documentation/DocBook/procfs_example.c write_proc/Documentation/DocBook/procfs_example.c
@@ -88,7 +88,8 @@
 static int proc_write_foobar(struct file *file,
 			     const char *buffer,
 			     unsigned long count, 
-			     void *data)
+			     void *data,
+			     off_t off)
 {
 	int len;
 	struct fb_data_t *fb_data = (struct fb_data_t *)data;

drivers/acpi/debug.c +1 -1

diff -u -N -r write_proc.orig/drivers/acpi/debug.c write_proc/drivers/acpi/debug.c
@@ -266,7 +266,7 @@
 static int
 acpi_system_write_debug(struct file *file,
 			const char __user * buffer,
-			unsigned long count, void *data)
+			unsigned long count, void *data, off_t off)
 {
 	char debug_string[12] = { '\0' };

drivers/block/DAC960.c +2 -1

diff -u -N -r write_proc.orig/drivers/block/DAC960.c write_proc/drivers/block/DAC960.c
@@ -6553,7 +6553,8 @@
 
 static int DAC960_ProcWriteUserCommand(struct file *file,
 				       const char __user *Buffer,
-				       unsigned long Count, void *Data)
+				       unsigned long Count, void *Data,
+				       off_t Offset)
 {
   DAC960_Controller_T *Controller = (DAC960_Controller_T *) Data;
   unsigned char CommandBuffer[80];

drivers/ide/ide-proc.c +4 -2

diff -u -N -r write_proc.orig/drivers/ide/ide-proc.c write_proc/drivers/ide/ide-proc.c
@@ -297,7 +297,8 @@
 #define MAX_LEN	30
 
 static int proc_ide_write_settings(struct file *file, const char __user *buffer,
-				   unsigned long count, void *data)
+				   unsigned long count, void *data,
+				   off_t off)
 {
 	ide_drive_t	*drive = (ide_drive_t *) data;
 	char		name[MAX_LEN + 1];

@@ -481,7 +482,8 @@
 }
 
 static int proc_ide_write_driver
-	(struct file *file, const char __user *buffer, unsigned long count, void *data)
+	(struct file *file, const char __user *buffer, unsigned long count,
+	 void *data, off_t off)
 {
 	ide_drive_t	*drive = (ide_drive_t *) data;
 	char name[32];

drivers/isdn/hardware/eicon/divasproc.c +4 -4

diff -u -N -r write_proc.orig/drivers/isdn/hardware/eicon/divasproc.c write_proc/drivers/isdn/hardware/eicon/divasproc.c
@@ -146,7 +146,7 @@
 */
 static int
 write_grp_opt(struct file *file, const char __user *buffer, unsigned long count,
-	      void *data)
+	      void *data, off_t off)
 {
 	diva_os_xdi_adapter_t *a = (diva_os_xdi_adapter_t *) data;
 	PISDN_ADAPTER IoAdapter = IoAdapters[a->controller - 1];

@@ -176,8 +176,8 @@
 ** write dynamic_l1_down
 */
 static int
-write_d_l1_down(struct file *file, const char __user *buffer, unsigned long count,
-		void *data)
+write_d_l1_down(struct file *file, const char __user *buffer,
+		unsigned long count, void *data, off_t off)
 {
 	diva_os_xdi_adapter_t *a = (diva_os_xdi_adapter_t *) data;
 	PISDN_ADAPTER IoAdapter = IoAdapters[a->controller - 1];

@@ -257,7 +257,7 @@
 */
 static int
 info_write(struct file *file, const char __user *buffer, unsigned long count,
-	   void *data)
+	   void *data, off_t off)
 {
 	diva_os_xdi_adapter_t *a = (diva_os_xdi_adapter_t *) data;
 	PISDN_ADAPTER IoAdapter = IoAdapters[a->controller - 1];

drivers/macintosh/via-pmu.c +2 -2

diff -u -N -r write_proc.orig/drivers/macintosh/via-pmu.c write_proc/drivers/macintosh/via-pmu.c
@@ -196,7 +196,7 @@
 static int proc_read_options(char *page, char **start, off_t off,
 			int count, int *eof, void *data);
 static int proc_write_options(struct file *file, const char __user *buffer,
-			unsigned long count, void *data);
+			unsigned long count, void *data, off_t off);
 
 #ifdef CONFIG_ADB
 struct adb_driver via_pmu_driver = {

@@ -880,7 +880,7 @@
 			
 static int
 proc_write_options(struct file *file, const char __user *buffer,
-			unsigned long count, void *data)
+			unsigned long count, void *data, off_t off)
 {
 	char tmp[33];
 	char *label, *val;

drivers/media/dvb/ttpci/av7110_ir.c +1 -1

diff -u -N -r write_proc.orig/drivers/media/dvb/ttpci/av7110_ir.c write_proc/drivers/media/dvb/ttpci/av7110_ir.c
@@ -269,7 +269,7 @@
 
 /* /proc/av7110_ir interface */
 static int av7110_ir_write_proc(struct file *file, const char __user *buffer,
-				unsigned long count, void *data)
+				unsigned long count, void *data, off_t off)
 {
 	char *page;
 	u32 ir_config;

drivers/media/video/cpia.c +1 -1

diff -u -N -r write_proc.orig/drivers/media/video/cpia.c write_proc/drivers/media/video/cpia.c
@@ -565,7 +565,7 @@
 }
 
 static int cpia_write_proc(struct file *file, const char __user *buf,
-			   unsigned long count, void *data)
+			   unsigned long count, void *data, off_t off)
 {
 	struct cam_data *cam = data;
 	struct cam_params new_params;

drivers/net/wireless/ipw2x00/libipw_module.c +1 -1

diff -u -N -r write_proc.orig/drivers/net/wireless/ipw2x00/libipw_module.c write_proc/drivers/net/wireless/ipw2x00/libipw_module.c
@@ -223,7 +223,7 @@
 }
 
 static int store_debug_level(struct file *file, const char __user * buffer,
-			     unsigned long count, void *data)
+			     unsigned long count, void *data, off_t off)
 {
 	char buf[] = "0x00000000\n";
 	unsigned long len = min((unsigned long)sizeof(buf) - 1, count);

drivers/net/wireless/ray_cs.c +2 -2

diff -u -N -r write_proc.orig/drivers/net/wireless/ray_cs.c write_proc/drivers/net/wireless/ray_cs.c
@@ -2875,7 +2875,7 @@
 }
 
 static int write_essid(struct file *file, const char __user *buffer,
-		       unsigned long count, void *data)
+		       unsigned long count, void *data, off_t off)
 {
 	static char proc_essid[33];
 	int len = count;

@@ -2890,7 +2890,7 @@
 }
 
 static int write_int(struct file *file, const char __user *buffer,
-		     unsigned long count, void *data)
+		     unsigned long count, void *data, off_t off)
 {
 	static char proc_number[10];
 	char *p;

drivers/parisc/led.c +1 -1

diff -u -N -r write_proc.orig/drivers/parisc/led.c write_proc/drivers/parisc/led.c
@@ -180,7 +180,7 @@
 }
 
 static int led_proc_write(struct file *file, const char *buf, 
-	unsigned long count, void *data)
+	unsigned long count, void *data, off_t off)
 {
 	char *cur, lbuf[count + 1];
 	int d;

drivers/platform/x86/asus_acpi.c +9 -9

diff -u -N -r write_proc.orig/drivers/platform/x86/asus_acpi.c write_proc/drivers/platform/x86/asus_acpi.c
@@ -648,7 +648,7 @@
 
 static int
 proc_write_mled(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	return write_led(buffer, count, hotk->methods->mt_mled, MLED_ON, 1);
 }

@@ -665,7 +665,7 @@
 
 static int
 proc_write_ledd(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t ppos)
 {
 	int rv, value;

@@ -694,7 +694,7 @@
 
 static int
 proc_write_wled(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	return write_led(buffer, count, hotk->methods->mt_wled, WLED_ON, 0);
 }

@@ -711,7 +711,7 @@
 
 static int
 proc_write_bluetooth(struct file *file, const char __user *buffer,
-		     unsigned long count, void *data)
+		     unsigned long count, void *data, off_t off)
 {
 	/* Note: mt_bt_switch controls both internal Bluetooth adapter's
 	   presence and its LED */

@@ -731,7 +731,7 @@
 
 static int
 proc_write_tled(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	return write_led(buffer, count, hotk->methods->mt_tled, TLED_ON, 0);
 }

@@ -837,7 +837,7 @@
 
 static int
 proc_write_lcd(struct file *file, const char __user *buffer,
-	       unsigned long count, void *data)
+	       unsigned long count, void *data, off_t off)
 {
 	int rv, value;

@@ -915,7 +915,7 @@
 
 static int
 proc_write_brn(struct file *file, const char __user *buffer,
-	       unsigned long count, void *data)
+	       unsigned long count, void *data, off_t off)
 {
 	int rv, value;

@@ -962,7 +962,7 @@
  */
 static int
 proc_write_disp(struct file *file, const char __user *buffer,
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	int rv, value;

@@ -975,7 +975,7 @@
 typedef int (proc_readfunc) (char *page, char **start, off_t off, int count,
 			     int *eof, void *data);
 typedef int (proc_writefunc) (struct file *file, const char __user *buffer,
-			      unsigned long count, void *data);
+			      unsigned long count, void *data, off_t off);
 
 static int
 asus_proc_add(char *name, proc_writefunc *writefunc,

drivers/platform/x86/thinkpad_acpi.c +1 -1

diff -u -N -r write_proc.orig/drivers/platform/x86/thinkpad_acpi.c write_proc/drivers/platform/x86/thinkpad_acpi.c
@@ -758,7 +758,7 @@
 
 static int dispatch_procfs_write(struct file *file,
 			const char __user *userbuf,
-			unsigned long count, void *data)
+			unsigned long count, void *data, off_t off)
 {
 	struct ibm_struct *ibm = data;
 	char *kernbuf;

drivers/platform/x86/toshiba_acpi.c +1 -1

diff -u -N -r write_proc.orig/drivers/platform/x86/toshiba_acpi.c write_proc/drivers/platform/x86/toshiba_acpi.c
@@ -390,7 +390,7 @@
 
 static int
 dispatch_write(struct file *file, const char __user * buffer,
-	       unsigned long count, ProcItem * item)
+	       unsigned long count, ProcItem * item, off_t off)
 {
 	int result;
 	char *tmp_buffer;

drivers/pnp/pnpbios/proc.c +1 -1

diff -u -N -r write_proc.orig/drivers/pnp/pnpbios/proc.c write_proc/drivers/pnp/pnpbios/proc.c
@@ -186,7 +186,7 @@
 }
 
 static int proc_write_node(struct file *file, const char __user * buf,
-			   unsigned long count, void *data)
+			   unsigned long count, void *data, off_t off)
 {
 	struct pnp_bios_node *node;
 	int boot = (long)data >> 8;

drivers/s390/block/dasd_proc.c +1 -1

diff -u -N -r write_proc.orig/drivers/s390/block/dasd_proc.c write_proc/drivers/s390/block/dasd_proc.c
@@ -259,7 +259,7 @@
 
 static int
 dasd_statistics_write(struct file *file, const char __user *user_buf,
-		      unsigned long user_len, void *data)
+		      unsigned long user_len, void *data, off_t off)
 {
 #ifdef CONFIG_DASD_PROFILE
 	char *buffer, *str;

drivers/s390/crypto/zcrypt_api.c +1 -1

diff -u -N -r write_proc.orig/drivers/s390/crypto/zcrypt_api.c write_proc/drivers/s390/crypto/zcrypt_api.c
@@ -1058,7 +1058,7 @@
 }
 
 static int zcrypt_status_write(struct file *file, const char __user *buffer,
-			       unsigned long count, void *data)
+			       unsigned long count, void *data, off_t off)
 {
 	unsigned char *lbuf, *ptr;
 	unsigned long local_count;

drivers/scsi/scsi_proc.c +1 -1

diff -u -N -r write_proc.orig/drivers/scsi/scsi_proc.c write_proc/drivers/scsi/scsi_proc.c
@@ -75,7 +75,7 @@
  * @data: pointer to &struct Scsi_Host
  */
 static int proc_scsi_write_proc(struct file *file, const char __user *buf,
-                           unsigned long count, void *data)
+                           unsigned long count, void *data, off_t off)
 {
 	struct Scsi_Host *shost = data;
 	ssize_t ret = -ENOMEM;

drivers/staging/epl/proc_fs.c +2 -2

diff -u -N -r write_proc.orig/drivers/staging/epl/proc_fs.c write_proc/drivers/staging/epl/proc_fs.c
@@ -144,7 +144,7 @@
 static int EplLinProcRead(char *pcBuffer_p, char **ppcStart_p, off_t Offset_p,
 			  int nBufferSize_p, int *pEof_p, void *pData_p);
 static int EplLinProcWrite(struct file *file, const char __user * buffer,
-			   unsigned long count, void *data);
+			   unsigned long count, void *data, off_t off);
 
 void TgtDbgSignalTracePoint(u8 bTracePointNumber_p);
 void TgtDbgPostTraceValue(u32 dwTraceValue_p);

@@ -382,7 +382,7 @@
 //---------------------------------------------------------------------------
 
 static int EplLinProcWrite(struct file *file, const char __user * buffer,
-			   unsigned long count, void *data)
+			   unsigned long count, void *data, off_t off)
 {
 	char abBuffer[count + 1];
 	int iErr;

drivers/staging/rtl8192su/ieee80211/ieee80211_module.c +1 -1

diff -u -N -r write_proc.orig/drivers/staging/rtl8192su/ieee80211/ieee80211_module.c write_proc/drivers/staging/rtl8192su/ieee80211/ieee80211_module.c
@@ -301,7 +301,7 @@
 }
 
 static int store_debug_level(struct file *file, const char *buffer,
-			     unsigned long count, void *data)
+			     unsigned long count, void *data, off_t off)
 {
 	char buf[] = "0x00000000";
 	unsigned long len = min(sizeof(buf) - 1, (u32)count);

drivers/video/clps711xfb.c +2 -2

diff -u -N -r write_proc.orig/drivers/video/clps711xfb.c write_proc/drivers/video/clps711xfb.c
@@ -44,7 +44,7 @@
 static int clps7111fb_proc_backlight_read(char *page, char **start, off_t off,
 		int count, int *eof, void *data);
 static int clps7111fb_proc_backlight_write(struct file *file, 
-		const char *buffer, unsigned long count, void *data);
+		const char *buffer, unsigned long count, void *data, off_t off);
 
 /*
  * LCD AC Prescale.  This comes from the LCD panel manufacturers specifications.

@@ -240,7 +240,7 @@
 
 static int 
 clps7111fb_proc_backlight_write(struct file *file, const char *buffer, 
-		unsigned long count, void *data)
+		unsigned long count, void *data, off_t off)
 {
 	unsigned char char_value;
 	int value;

drivers/video/via/viafbdev.c +10 -5

diff -u -N -r write_proc.orig/drivers/video/via/viafbdev.c write_proc/drivers/video/via/viafbdev.c
@@ -1772,7 +1772,8 @@
 	return len;
 }
 static int viafb_dvp0_proc_write(struct file *file,
-	const char __user *buffer, unsigned long count, void *data)
+	const char __user *buffer, unsigned long count, void *data,
+	off_t offset)
 {
 	char buf[20], *value, *pbuf;
 	u8 reg_val = 0;

@@ -1830,7 +1831,8 @@
 	return len;
 }
 static int viafb_dvp1_proc_write(struct file *file,
-	const char __user *buffer, unsigned long count, void *data)
+	const char __user *buffer, unsigned long count, void *data,
+	off_t offset)
 {
 	char buf[20], *value, *pbuf;
 	u8 reg_val = 0;

@@ -1880,7 +1882,8 @@
 	return len;
 }
 static int viafb_dfph_proc_write(struct file *file,
-	const char __user *buffer, unsigned long count, void *data)
+	const char __user *buffer, unsigned long count, void *data,
+	off_t offset)
 {
 	char buf[20];
 	u8 reg_val = 0;

@@ -1906,7 +1909,8 @@
 	return len;
 }
 static int viafb_dfpl_proc_write(struct file *file,
-	const char __user *buffer, unsigned long count, void *data)
+	const char __user *buffer, unsigned long count, void *data,
+	off_t offset)
 {
 	char buf[20];
 	u8 reg_val = 0;

@@ -1956,7 +1960,8 @@
 	return len;
 }
 static int viafb_vt1636_proc_write(struct file *file,
-	const char __user *buffer, unsigned long count, void *data)
+	const char __user *buffer, unsigned long count, void *data,
+	off_t offset)
 {
 	char buf[30], *value, *pbuf;
 	struct IODATA reg_val;

kernel/profile.c +1 -1

diff -u -N -r write_proc.orig/kernel/profile.c write_proc/kernel/profile.c
@@ -455,7 +455,7 @@
 }
 
 static int prof_cpu_mask_write_proc(struct file *file,
-	const char __user *buffer,  unsigned long count, void *data)
+	const char __user *buffer,  unsigned long count, void *data, off_t off)
 {
 	struct cpumask *mask = data;
 	unsigned long full_count = count, err;

Documentation/DocBook/procfs-guide.tmpl +1 -0

diff -u -N -r write_proc.orig/Documentation/DocBook/procfs-guide.tmpl write_proc/Documentation/DocBook/procfs-guide.tmpl
@@ -455,6 +455,7 @@
 	  <paramdef>const char* <parameter>buffer</parameter></paramdef>
 	  <paramdef>unsigned long <parameter>count</parameter></paramdef>
 	  <paramdef>void* <parameter>data</parameter></paramdef>
+	  <paramdef>off_t <parameter>off</parameter></paramdef>
 	</funcprototype>
       </funcsynopsis>

Christoph Hellwig

2009-08-29 23:16 UTC #1 lore raw

Quoted 2 lines · Stefani Seibold

The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.

The right fix is to get rid of the last remaining read_proc_t/write_proc_t instances and switch everyone to implement file operations. Alexey has been working on this on and off for a while.

Alexey Dobriyan

2009-08-30 08:09 UTC #2 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 2 lines · Stefani Seibold
The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.
The right fix is to get rid of the last remaining read_proc_t/write_proc_t instances and switch everyone to implement file operations. Alexey has been working on this on and off for a while.

Yes, please, ->write_proc is going to end very soon.

As for prevoius arguments:
- changing ->write_proc signature is _of course_ going to break out-of-tree stuff
- ->proc_fops worked since forever, and since it's struct file_operations, nobody prohibits implementing ->write with pos update and what not.
- seq_file is not relevant to this issue because seq_file is for read path only.

Back into SCSI pile of users.

Stefani Seibold

author

2009-08-30 19:10 UTC #3 lore raw

Am Sonntag, den 30.08.2009, 12:09 +0400 schrieb Alexey Dobriyan:

Quoted 9 lines · Christoph Hellwig

Quoted 7 lines · Stefani Seibold
Quoted 2 lines
The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.
The right fix is to get rid of the last remaining read_proc_t/write_proc_t instances and switch everyone to implement file operations. Alexey has been working on this on and off for a while.

Quoted 5 lines

Yes, please, ->write_proc is going to end very soon.
As for prevoius arguments:
- changing ->write_proc signature is _of course_ going to break out-of-tree stuff

No, it breaks nothing. You get only a warning. I think thats okay...

Quoted 6 lines

- ->proc_fops worked since forever, and since it's struct file_operations, nobody prohibits implementing ->write with pos update and what not.
- seq_file is not relevant to this issue because seq_file is for read path only.
Back into SCSI pile of users.

Stefani Seibold

author

2009-08-30 19:05 UTC #4 lore raw

In reply to Christoph Hellwig

Am Samstag, den 29.08.2009, 19:16 -0400 schrieb Christoph Hellwig:

Quoted 8 lines · Stefani Seibold

Quoted 2 lines
The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.
The right fix is to get rid of the last remaining read_proc_t/write_proc_t instances and switch everyone to implement file operations. Alexey has been working on this on and off for a while.

Switching all users of read_proc_t/write_proc_t to file operation is a huge job. About 180 files must be fixed.

But the main reason not to do this is because the breakage of "out of tree" drivers.

I like the current simplified proc interface. It saves a lot of code duplication because the basic operations will be handled inside the kernel and not in the driver.

There is no reason for Alexey to finish his work, submit it and maybe it will be accepted.

In the meantime i will see my patch as the preferred solution: a cleanup of the current interface.

Alexey Dobriyan

2009-08-31 06:33 UTC #5 lore raw

Quoted 5 lines · Stefani Seibold

Am Samstag, den 29.08.2009, 19:16 -0400 schrieb Christoph Hellwig:
Quoted 4 lines · Stefani Seibold
Quoted 2 lines
The following fix a long standing issue in the proc_file_write function, which doesn't update the ppos file position pointer.

Quoted 7 lines

Quoted 4 lines
The right fix is to get rid of the last remaining read_proc_t/write_proc_t instances and switch everyone to implement file operations. Alexey has been working on this on and off for a while.
Switching all users of read_proc_t/write_proc_t to file operation is a huge job. About 180 files must be fixed.

Less, staging stuff is on it's own.

Quoted 2 lines

But the main reason not to do this is because the breakage of "out of tree" drivers.

Just how many times it needs to be repeated.

Out-of-tree argument means almost nothing.

Quoted 3 lines

I like the current simplified proc interface. It saves a lot of code duplication because the basic operations will be handled inside the kernel and not in the driver.

It's garbage.

Arbirtrary 3 KB limit on read,
kernel memory giving trailer in ->read_proc which is impossible to get right, was always buggy wrt module unload (quickly fixed, but it was always meant to be temporary).

Now that SCSI pile is mostly finished, what a horrible code interface provoked.

Quoted 2 lines

There is no reason for Alexey to finish his work, submit it and maybe it will be accepted.

You'd better start converting to struct file_operations::write now.

Quoted 2 lines

In the meantime i will see my patch as the preferred solution: a cleanup of the current interface.

Arnd Bergmann

2009-08-31 15:45 UTC #6 lore raw

Quoted 5 lines · Alexey Dobriyan

Out-of-tree argument means almost nothing.
...
You'd better start converting to struct file_operations::write now.

Maybe a purely mechanical conversion to file_operations would be a nice first step, just so we can remove read_proc and write_proc? Taking scsi_proc.c as an example, this should be really straightforward and lets us get rid of the write_proc and read_proc callbacks from proc_dir_entry without having to rewrite all the remaining drivers that use it.

Obviously, someone who understands the specific driver code better should then clean up the code by converting to seq_file operations or something else that is appropriate there.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/scsi/scsi_proc.c |   27 ++++++++++++++++++++++-----
 fs/proc/generic.c        |   16 ++++++----------
 include/linux/proc_fs.h  |   11 +++++++++++
 3 files changed, 39 insertions(+), 15 deletions(-)

drivers/scsi/scsi_proc.c +23 -5

index 77fbddb..f2a48a1 100644
@@ -136,6 +136,26 @@ void scsi_proc_hostdir_rm(struct scsi_host_template *sht)
 	mutex_unlock(&global_host_template_mutex);
 }
 
+static ssize_t scsi_proc_read(struct file *file, char __user *buf,
+			size_t count, loff_t *off)
+{
+	return __proc_file_read(&proc_scsi_read, buf, count, off,
+			PDE(file->f_path.dentry->d_inode)->data);
+}
+
+static ssize_t scsi_proc_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *off)
+{
+	return proc_scsi_write_proc(file, buf, count,
+			PDE(file->f_path.dentry->d_inode)->data);
+}
+
+static const struct file_operations scsi_proc_fops = {
+	.owner = THIS_MODULE,
+	.read = scsi_proc_read,
+	.write = scsi_proc_write,
+	.llseek = generic_file_llseek,
+};
 
 /**
  * scsi_proc_host_add - Add entry for this host to appropriate /proc dir

@@ -151,16 +170,14 @@ void scsi_proc_host_add(struct Scsi_Host *shost)
 		return;
 
 	sprintf(name,"%d", shost->host_no);
-	p = create_proc_read_entry(name, S_IFREG | S_IRUGO | S_IWUSR,
-			sht->proc_dir, proc_scsi_read, shost);
+	p = proc_create_data(name, S_IFREG | S_IRUGO | S_IWUSR,
+			sht->proc_dir, &scsi_proc_fops, shost);
 	if (!p) {
 		printk(KERN_ERR "%s: Failed to register host %d in"
 		       "%s\n", __func__, shost->host_no,
 		       sht->proc_name);
 		return;
-	} 
-
-	p->write_proc = proc_scsi_write_proc;
+	}
 }
 
 /**

fs/proc/generic.c +6 ~~-10~~

index fa678ab..ccf3949 100644
@@ -36,17 +36,14 @@ static int proc_match(int len, const char *name, struct proc_dir_entry *de)
 /* buffer size is one page but our output routines use some slack for overruns */
 #define PROC_BLOCK_SIZE	(PAGE_SIZE - 1024)
 
-static ssize_t
-__proc_file_read(struct file *file, char __user *buf, size_t nbytes,
-	       loff_t *ppos)
+ssize_t __proc_file_read(read_proc_t read_proc, char __user *buf,
+			size_t nbytes, loff_t *ppos, void *data)
 {
-	struct inode * inode = file->f_path.dentry->d_inode;
 	char 	*page;
 	ssize_t	retval=0;
 	int	eof=0;
 	ssize_t	n, count;
 	char	*start;
-	struct proc_dir_entry * dp;
 	unsigned long long pos;
 
 	/*

@@ -60,7 +57,6 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 	if (nbytes > MAX_NON_LFS - pos)
 		nbytes = MAX_NON_LFS - pos;
 
-	dp = PDE(inode);
 	if (!(page = (char*) __get_free_page(GFP_TEMPORARY)))
 		return -ENOMEM;

@@ -68,7 +64,7 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 		count = min_t(size_t, PROC_BLOCK_SIZE, nbytes);
 
 		start = NULL;
-		if (dp->read_proc) {
+		if (read_proc) {
 			/*
 			 * How to be a proc read function
 			 * ------------------------------

@@ -116,8 +112,8 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 			 *    requested offset advanced by the number of bytes
 			 *    absorbed.
 			 */
-			n = dp->read_proc(page, &start, *ppos,
-					  count, &eof, dp->data);
+			n = read_proc(page, &start, *ppos,
+					  count, &eof, data);
 		} else
 			break;

@@ -197,7 +193,7 @@ proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 	pde->pde_users++;
 	spin_unlock(&pde->pde_unload_lock);
 
-	rv = __proc_file_read(file, buf, nbytes, ppos);
+	rv = __proc_file_read(pde->read_proc, buf, nbytes, ppos, pde->data);
 
 	pde_users_dec(pde);
 	return rv;

include/linux/proc_fs.h +11 -0

index e6e77d3..949a382 100644
@@ -103,6 +103,10 @@ struct proc_dir_entry *proc_create_data(const char *name, mode_t mode,
 				struct proc_dir_entry *parent,
 				const struct file_operations *proc_fops,
 				void *data);
+/* do not use in new code */
+ssize_t __proc_file_read(read_proc_t read_proc, char __user *buf,
+                        size_t nbytes, loff_t *ppos, void *data);
+
 extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent);
 
 struct pid_namespace;

@@ -195,6 +199,13 @@ static inline struct proc_dir_entry *proc_create_data(const char *name,
 }
 #define remove_proc_entry(name, parent) do {} while (0)
 
+static inline ssize_t __proc_file_read(read_proc_t read_proc,
+			char __user *buf, size_t nbytes, loff_t *ppos,
+			void *data)
+{
+	return 0;
+}
+
 static inline struct proc_dir_entry *proc_symlink(const char *name,
 		struct proc_dir_entry *parent,const char *dest) {return NULL;}
 static inline struct proc_dir_entry *proc_mkdir(const char *name,

Alexey Dobriyan

2009-08-31 17:19 UTC #7 lore raw

Quoted 13 lines · Arnd Bergmann

Quoted 5 lines · Alexey Dobriyan
Out-of-tree argument means almost nothing.
...
You'd better start converting to struct file_operations::write now.
Maybe a purely mechanical conversion to file_operations would be a nice first step, just so we can remove read_proc and write_proc? Taking scsi_proc.c as an example, this should be really straightforward and lets us get rid of the write_proc and read_proc callbacks from proc_dir_entry without having to rewrite all the remaining drivers that use it.

Quoted 3 lines

Obviously, someone who understands the specific driver code better should then clean up the code by converting to seq_file operations or something else that is appropriate there.

I don't like any churn in that area. Everything is ready for ->proc_fops usage. So we should simply convert users away.
Actually most of SCSI and IDE is already done, just wasn't sent yet.

After conversion, ->read_proc, ->write_proc, create_proc_read_entry(), create_proc_entry() will become deprecated for a while, then gone.

Christoph Hellwig

2009-08-31 17:21 UTC #8 lore raw

Quoted 2 lines · Alexey Dobriyan

After conversion, ->read_proc, ->write_proc, create_proc_read_entry(), create_proc_entry() will become deprecated for a while, then gone.

Just kill it once all intree users are gone, deprecation is only useful if we still have some in-tree user around but want to keep people from adding new ones.

Arnd Bergmann

2009-08-31 20:19 UTC #9 lore raw

In reply to Alexey Dobriyan

Quoted 3 lines · Alexey Dobriyan

I don't like any churn in that area. Everything is ready for ->proc_fops usage. So we should simply convert users away.
Actually most of SCSI and IDE is already done, just wasn't sent yet.

Ok, fair enough. I was expecting that the remaining drivers are suffering from lack of testing options, as many of them are rather obscure. If you think we can just convert them in one step, that's even better.

	Arnd <><

Al Viro

2009-09-12 15:28 UTC #10 lore raw

In reply to Stefani Seibold

Quoted 9 lines · Stefani Seibold

Switching all users of read_proc_t/write_proc_t to file operation is a huge job. About 180 files must be fixed.
But the main reason not to do this is because the breakage of "out of tree" drivers.
I like the current simplified proc interface. It saves a lot of code duplication because the basic operations will be handled inside the kernel and not in the driver.

_What_ code duplication? Would that be

        struct proc_dir_entry *pde = PDE(file->f_path.dentry->d_inode);

and calculation of pde->data currently done by proc_file_write()? Pardon me, but that's hard to take seriously. As for the ->read() side, most of those suckers end up switched to use of seq_read() and there's not a lot of boilerplate code in the resulting variants...

Stefani Seibold

author

2009-09-12 15:57 UTC #11 lore raw

Am Samstag, den 12.09.2009, 16:28 +0100 schrieb Al Viro:

Quoted 11 lines · Stefani Seibold

Quoted 9 lines
Switching all users of read_proc_t/write_proc_t to file operation is a huge job. About 180 files must be fixed.
But the main reason not to do this is because the breakage of "out of tree" drivers.
I like the current simplified proc interface. It saves a lot of code duplication because the basic operations will be handled inside the kernel and not in the driver.

Quoted 6 lines

_What_ code duplication? Would that be
        struct proc_dir_entry *pde = PDE(file->f_path.dentry->d_inode);
and calculation of pde->data currently done by proc_file_write()? Pardon me, but that's hard to take seriously. As for the ->read() side, most of those suckers end up switched to use of seq_read() and there's not a lot of boilerplate code in the resulting variants...

Hi Al, you are 2 weeks to late, the discussion has already ended. But lets start it again....

There is some code duplication, proc_file_write has 17 lines of code. And together with my path it will be 19 lines of code. How much lines of duplicated code are necessary for take them seriously?

I know that the current mainstream development targets enterprise and maybe sometime desktop systems. But the truth is that linux will be mostly used in embedded system with very limited memory constrains.

BTW: I personally think the whole seq_.... interface is IMHO to complicate to use, procfs is much simpler. But this is only my personal opinion!

Greetings,
Stefani

Eric W. Biederman

2009-09-12 20:51 UTC #12 lore raw

Quoted 26 lines · Stefani Seibold

Am Samstag, den 12.09.2009, 16:28 +0100 schrieb Al Viro:
Quoted 11 lines · Stefani Seibold
Quoted 9 lines
Switching all users of read_proc_t/write_proc_t to file operation is a huge job. About 180 files must be fixed.
But the main reason not to do this is because the breakage of "out of tree" drivers.
I like the current simplified proc interface. It saves a lot of code duplication because the basic operations will be handled inside the kernel and not in the driver.
Quoted 6 lines
_What_ code duplication? Would that be
        struct proc_dir_entry *pde = PDE(file->f_path.dentry->d_inode);
and calculation of pde->data currently done by proc_file_write()? Pardon me, but that's hard to take seriously. As for the ->read() side, most of those suckers end up switched to use of seq_read() and there's not a lot of boilerplate code in the resulting variants...
Hi Al, you are 2 weeks to late, the discussion has already ended. But lets start it again....
There is some code duplication, proc_file_write has 17 lines of code. And together with my path it will be 19 lines of code. How much lines of duplicated code are necessary for take them seriously?

Quoted 7 lines

I know that the current mainstream development targets enterprise and maybe sometime desktop systems. But the truth is that linux will be mostly used in embedded system with very limited memory constrains.
BTW: I personally think the whole seq_.... interface is IMHO to complicate to use, procfs is much simpler. But this is only my personal opinion!

The point of all of this is that you are trying to enhance an interface that is essentially deprecated. We don't want to use it anymore because at least on the read side there are recurring problems with buffer overflows and reads not handling seeks properly, etc. So everything is slowing switching being fixed and taken off of the current proc_file_read/proc_file_write handlers.

Eric