All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy)
@ 2010-04-09 14:00 Jan Blunck
  2010-04-09 14:00 ` [PATCH 01/12] osst: Update ppos instead of using file->f_pos Jan Blunck
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

This is a repost of a patch series I already posted in November 2009. This time
I left out the patch that removes the BKL from default_llseek() since this
should go through Frederic's tree. This series mostly consists of fixing direct
modifications of file->f_pos and some additional cases where nonseekable_open()
can be used. The rest of this series is unrelated to but a preparation for the
BKL removal from default_llseek().

Thanks,
Jan

Jan Blunck (12):
  osst: Update ppos instead of using file->f_pos
  flash_read should update ppos instead of file->f_pos
  eeprom_read()/eeprom_write() should update ppos instead of
    file->f_pos
  frv: remove "struct file *" argument from sysctl ->proc_handler
  mISDN: Remove unnecessary test on f_pos
  zcrypt: Use nonseekable_open()
  rtc-m41t80: Use nonseekable_open()
  Introduce noop_llseek()
  osst: Use noop_llseek() instead of default_llseek()
  st: Use noop_llseek() instead of default_llseek()
  Do not fallback to default_llseek() when readdir() uses BKL
  Documentation/filesystems/Locking: Update documentation on llseek()
    wrt BKL

 Documentation/filesystems/Locking   |    5 +++--
 arch/cris/arch-v10/drivers/eeprom.c |   34 +++++++++++++---------------------
 arch/frv/kernel/sysctl.c            |   18 ++++++++++--------
 drivers/isdn/mISDN/timerdev.c       |    2 --
 drivers/rtc/rtc-m41t80.c            |    6 +-----
 drivers/s390/crypto/zcrypt_api.c    |    2 +-
 drivers/sbus/char/flash.c           |    6 +++---
 drivers/scsi/osst.c                 |    9 +++++----
 drivers/scsi/st.c                   |    1 +
 fs/autofs/root.c                    |    1 +
 fs/freevxfs/vxfs_lookup.c           |    2 ++
 fs/isofs/dir.c                      |    1 +
 fs/ncpfs/dir.c                      |    1 +
 fs/qnx4/dir.c                       |    1 +
 fs/read_write.c                     |   17 +++++++++++++++++
 fs/reiserfs/dir.c                   |    1 +
 fs/smbfs/dir.c                      |    1 +
 fs/udf/dir.c                        |    1 +
 include/linux/fs.h                  |    1 +
 19 files changed, 64 insertions(+), 46 deletions(-)


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

* [PATCH 01/12] osst: Update ppos instead of using file->f_pos
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 02/12] flash_read should update ppos instead of file->f_pos Jan Blunck
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

osst_read()/osst_write() modify file->f_pos directly instead of the ppos
given to them. The VFS later updates the file->f_pos and overwrites it
with the value of ppos.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/scsi/osst.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index acb8358..9dc0b9a 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -3586,7 +3586,7 @@ if (SRpnt) printk(KERN_ERR "%s:A: Not supposed to have SRpnt at line %d\n", name
 		if (i == (-ENOSPC)) {
 			transfer = STp->buffer->writing;	/* FIXME -- check this logic */
 			if (transfer <= do_count) {
-				filp->f_pos += do_count - transfer;
+				*ppos += do_count - transfer;
 				count -= do_count - transfer;
 				if (STps->drv_block >= 0) {
 					STps->drv_block += (do_count - transfer) / STp->block_size;
@@ -3624,7 +3624,7 @@ if (SRpnt) printk(KERN_ERR "%s:A: Not supposed to have SRpnt at line %d\n", name
 			goto out;
 		}
 
-		filp->f_pos += do_count;
+		*ppos += do_count;
 		b_point += do_count;
 		count -= do_count;
 		if (STps->drv_block >= 0) {
@@ -3646,7 +3646,7 @@ if (SRpnt) printk(KERN_ERR "%s:A: Not supposed to have SRpnt at line %d\n", name
 		if (STps->drv_block >= 0) {
 			STps->drv_block += blks;
 		}
-		filp->f_pos += count;
+		*ppos += count;
 		count = 0;
 	}
 
@@ -3822,7 +3822,7 @@ static ssize_t osst_read(struct file * filp, char __user * buf, size_t count, lo
 			}
 			STp->logical_blk_num += transfer / STp->block_size;
 			STps->drv_block      += transfer / STp->block_size;
-			filp->f_pos          += transfer;
+			*ppos          += transfer;
 			buf                  += transfer;
 			total                += transfer;
 		}
-- 
1.6.4.2


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

* [PATCH 02/12] flash_read should update ppos instead of file->f_pos
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
  2010-04-09 14:00 ` [PATCH 01/12] osst: Update ppos instead of using file->f_pos Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 03/12] eeprom_read()/eeprom_write() " Jan Blunck
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

flash_read() updates file->f_pos directly instead of the ppos given. The
VFS later updates the file->f_pos and overwrites it with the unchanged
value of ppos.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/sbus/char/flash.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/sbus/char/flash.c b/drivers/sbus/char/flash.c
index 4108347..12cd438 100644
--- a/drivers/sbus/char/flash.c
+++ b/drivers/sbus/char/flash.c
@@ -106,9 +106,9 @@ static ssize_t
 flash_read(struct file * file, char __user * buf,
 	   size_t count, loff_t *ppos)
 {
-	unsigned long p = file->f_pos;
+	loff_t p = *ppos;
 	int i;
-	
+
 	if (count > flash.read_size - p)
 		count = flash.read_size - p;
 
@@ -119,7 +119,7 @@ flash_read(struct file * file, char __user * buf,
 		buf++;
 	}
 
-	file->f_pos += count;
+	*ppos += count;
 	return count;
 }
 
-- 
1.6.4.2


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

* [PATCH 03/12] eeprom_read()/eeprom_write() should update ppos instead of file->f_pos
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
  2010-04-09 14:00 ` [PATCH 01/12] osst: Update ppos instead of using file->f_pos Jan Blunck
  2010-04-09 14:00 ` [PATCH 02/12] flash_read should update ppos instead of file->f_pos Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler Jan Blunck
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

eeprom_read()/eeprom_write() modify file->f_pos directly instead of the
ppos given. The VFS later updates the file->f_pos and overwrites it with
the unchanged value of ppos. Since nothing touches the struct file in
eeprom_read()/eeprom_write() now, the on stack allocated struct file in
eeprom_read_buf()/eeprom_write_buf() can be removed as well.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 arch/cris/arch-v10/drivers/eeprom.c |   34 +++++++++++++---------------------
 1 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/arch/cris/arch-v10/drivers/eeprom.c b/arch/cris/arch-v10/drivers/eeprom.c
index 1f2ae90..68d9e88 100644
--- a/arch/cris/arch-v10/drivers/eeprom.c
+++ b/arch/cris/arch-v10/drivers/eeprom.c
@@ -439,20 +439,18 @@ static loff_t eeprom_lseek(struct file * file, loff_t offset, int orig)
 
 static int eeprom_read_buf(loff_t addr, char * buf, int count)
 {
-  struct file f;
-
-  f.f_pos = addr;
-  return eeprom_read(&f, buf, count, &addr);
+  return eeprom_read(NULL, buf, count, &addr);
 }
 
 
 
 /* Reads data from eeprom. */
 
-static ssize_t eeprom_read(struct file * file, char * buf, size_t count, loff_t *off)
+static ssize_t eeprom_read(struct file * file, char * buf, size_t count,
+			   loff_t *ppos)
 {
   int read=0;
-  unsigned long p = file->f_pos;
+  loff_t p = *ppos;
 
   unsigned char page;
 
@@ -498,11 +496,9 @@ static ssize_t eeprom_read(struct file * file, char * buf, size_t count, loff_t
 
   /* go on with the actual read */
   read = read_from_eeprom( buf, count);
-  
+
   if(read > 0)
-  {
-    file->f_pos += read;
-  }
+    *ppos += read;
 
   eeprom.busy--;
   wake_up_interruptible(&eeprom.wait_q);
@@ -513,18 +509,14 @@ static ssize_t eeprom_read(struct file * file, char * buf, size_t count, loff_t
 
 static int eeprom_write_buf(loff_t addr, const char * buf, int count)
 {
-  struct file f;
-
-  f.f_pos = addr;
-  
-  return eeprom_write(&f, buf, count, &addr);
+  return eeprom_write(NULL, buf, count, &addr);
 }
 
 
 /* Writes data to eeprom. */
 
 static ssize_t eeprom_write(struct file * file, const char * buf, size_t count,
-                            loff_t *off)
+			    loff_t *ppos)
 {
   int i, written, restart=1;
   unsigned long p;
@@ -543,9 +535,9 @@ static ssize_t eeprom_write(struct file * file, const char * buf, size_t count,
   {
     restart = 0;
     written = 0;
-    p = file->f_pos;
-   
-    
+    p = *ppos;
+
+
     while( (written < count) && (p < eeprom.size))
     {
       /* address the eeprom */
@@ -671,10 +663,10 @@ static ssize_t eeprom_write(struct file * file, const char * buf, size_t count,
 
   eeprom.busy--;
   wake_up_interruptible(&eeprom.wait_q);
-  if (written == 0 && file->f_pos >= eeprom.size){
+  if (written == 0 && *ppos >= eeprom.size){
     return -ENOSPC;
   }
-  file->f_pos += written;
+  *ppos += written;
   return written;
 }
 
-- 
1.6.4.2


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

* [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (2 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 03/12] eeprom_read()/eeprom_write() " Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 16:23   ` Alexey Dobriyan
  2010-04-09 14:00 ` [PATCH 05/12] mISDN: Remove unnecessary test on f_pos Jan Blunck
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

Seems that Alexey Dobriyan missed this usage of the file argument when
removing it from ->proc_handler in commit 8d65af789f3e2cf4cfbdbf71a0f7a61ebcd41d38.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 arch/frv/kernel/sysctl.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/frv/kernel/sysctl.c b/arch/frv/kernel/sysctl.c
index 035516c..05afc05 100644
--- a/arch/frv/kernel/sysctl.c
+++ b/arch/frv/kernel/sysctl.c
@@ -47,8 +47,9 @@ static void frv_change_dcache_mode(unsigned long newmode)
 /*
  * handle requests to dynamically switch the write caching mode delivered by /proc
  */
-static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
-				 void __user *buffer, size_t *lenp, loff_t *ppos)
+static int procctl_frv_cachemode(ctl_table *table, int write,
+				 void __user *buffer, size_t *lenp,
+				 loff_t *ppos)
 {
 	unsigned long hsr0;
 	char buff[8];
@@ -85,7 +86,7 @@ static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
 	}
 
 	/* read the state */
-	if (filp->f_pos > 0) {
+	if (*ppos > 0) {
 		*lenp = 0;
 		return 0;
 	}
@@ -111,7 +112,7 @@ static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
 		return -EFAULT;
 
 	*lenp = len;
-	filp->f_pos = len;
+	*ppos = len;
 	return 0;
 
 } /* end procctl_frv_cachemode() */
@@ -121,8 +122,9 @@ static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
  * permit the mm_struct the nominated process is using have its MMU context ID pinned
  */
 #ifdef CONFIG_MMU
-static int procctl_frv_pin_cxnr(ctl_table *table, int write, struct file *filp,
-				void __user *buffer, size_t *lenp, loff_t *ppos)
+static int procctl_frv_pin_cxnr(ctl_table *table, int write,
+				void __user *buffer, size_t *lenp,
+				loff_t *ppos)
 {
 	pid_t pid;
 	char buff[16], *p;
@@ -151,7 +153,7 @@ static int procctl_frv_pin_cxnr(ctl_table *table, int write, struct file *filp,
 	}
 
 	/* read the currently pinned CXN */
-	if (filp->f_pos > 0) {
+	if (*ppos > 0) {
 		*lenp = 0;
 		return 0;
 	}
@@ -164,7 +166,7 @@ static int procctl_frv_pin_cxnr(ctl_table *table, int write, struct file *filp,
 		return -EFAULT;
 
 	*lenp = len;
-	filp->f_pos = len;
+	*ppos = len;
 	return 0;
 
 } /* end procctl_frv_pin_cxnr() */
-- 
1.6.4.2


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

* [PATCH 05/12] mISDN: Remove unnecessary test on f_pos
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (3 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 06/12] zcrypt: Use nonseekable_open() Jan Blunck
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

This test is not doing anything since it is always false if the mISDN_read()
is called from vfs_read(). Besides that the driver uses nonseekable_open()
and is not using off or file->f_pos anywhere.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/isdn/mISDN/timerdev.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/mISDN/timerdev.c b/drivers/isdn/mISDN/timerdev.c
index 5b7e9bf..0a8b440 100644
--- a/drivers/isdn/mISDN/timerdev.c
+++ b/drivers/isdn/mISDN/timerdev.c
@@ -96,8 +96,6 @@ mISDN_read(struct file *filep, char __user *buf, size_t count, loff_t *off)
 	if (*debug & DEBUG_TIMER)
 		printk(KERN_DEBUG "%s(%p, %p, %d, %p)\n", __func__,
 			filep, buf, (int)count, off);
-	if (*off != filep->f_pos)
-		return -ESPIPE;
 
 	if (list_empty(&dev->expired) && (dev->work == 0)) {
 		if (filep->f_flags & O_NONBLOCK)
-- 
1.6.4.2


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

* [PATCH 06/12] zcrypt: Use nonseekable_open()
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (4 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 05/12] mISDN: Remove unnecessary test on f_pos Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 07/12] rtc-m41t80: " Jan Blunck
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

zcrypt doesn't even allow to read/write the files it opens. So seeking on
it shouldn't fall back to defaults either but should be disallowed as well.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/s390/crypto/zcrypt_api.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index ba50fe0..18a986d 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -301,7 +301,7 @@ static ssize_t zcrypt_write(struct file *filp, const char __user *buf,
 static int zcrypt_open(struct inode *inode, struct file *filp)
 {
 	atomic_inc(&zcrypt_open_count);
-	return 0;
+	return nonseekable_open(inode, filp);
 }
 
 /**
-- 
1.6.4.2


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

* [PATCH 07/12] rtc-m41t80: Use nonseekable_open()
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (5 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 06/12] zcrypt: Use nonseekable_open() Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 08/12] Introduce noop_llseek() Jan Blunck
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

Use nonseekable_open() for this since seeking is not supported anyway.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/rtc/rtc-m41t80.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 60fe266..1e2ed76 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -595,10 +595,6 @@ static void wdt_disable(void)
 static ssize_t wdt_write(struct file *file, const char __user *buf,
 			 size_t count, loff_t *ppos)
 {
-	/*  Can't seek (pwrite) on this device
-	if (ppos != &file->f_pos)
-	return -ESPIPE;
-	*/
 	if (count) {
 		wdt_ping();
 		return 1;
@@ -695,7 +691,7 @@ static int wdt_open(struct inode *inode, struct file *file)
 		 */
 		wdt_is_open = 1;
 		unlock_kernel();
-		return 0;
+		return nonseekable_open(inode, file);
 	}
 	return -ENODEV;
 }
-- 
1.6.4.2


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

* [PATCH 08/12] Introduce noop_llseek()
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (6 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 07/12] rtc-m41t80: " Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 09/12] osst: Use noop_llseek() instead of default_llseek() Jan Blunck
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

This is an implementation of ->llseek useable for the rare special case
when userspace expects the seek to succeed but the (device) file is
actually not able to perform the seek. In this case you use noop_llseek()
instead of falling back to the default implementation of ->llseek.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 fs/read_write.c    |   17 +++++++++++++++++
 include/linux/fs.h |    1 +
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 113386d..9c04852 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -97,6 +97,23 @@ loff_t generic_file_llseek(struct file *file, loff_t offset, int origin)
 }
 EXPORT_SYMBOL(generic_file_llseek);
 
+/**
+ * noop_llseek - No Operation Performed llseek implementation
+ * @file:	file structure to seek on
+ * @offset:	file offset to seek to
+ * @origin:	type of seek
+ *
+ * This is an implementation of ->llseek useable for the rare special case when
+ * userspace expects the seek to succeed but the (device) file is actually not
+ * able to perform the seek. In this case you use noop_llseek() instead of
+ * falling back to the default implementation of ->llseek.
+ */
+loff_t noop_llseek(struct file *file, loff_t offset, int origin)
+{
+	return file->f_pos;
+}
+EXPORT_SYMBOL(noop_llseek);
+
 loff_t no_llseek(struct file *file, loff_t offset, int origin)
 {
 	return -ESPIPE;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 10b8ded..067681c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2227,6 +2227,7 @@ extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
 
 extern void
 file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping);
+extern loff_t noop_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t no_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t generic_file_llseek(struct file *file, loff_t offset, int origin);
 extern loff_t generic_file_llseek_unlocked(struct file *file, loff_t offset,
-- 
1.6.4.2


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

* [PATCH 09/12] osst: Use noop_llseek() instead of default_llseek()
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (7 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 08/12] Introduce noop_llseek() Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 10/12] st: " Jan Blunck
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

__os_scsi_tape_open() suggests that llseek() doesn't work:
"We really want to do nonseekable_open(inode, filp); here, but some
versions of tar incorrectly call lseek on tapes and bail out if that
fails.  So we disallow pread() and pwrite(), but permit lseeks."

Instead of using the fallback default_llseek() the driver should use
noop_llseek() which leaves the file->f_pos untouched but succeeds.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/scsi/osst.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 9dc0b9a..960e39c 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -5619,6 +5619,7 @@ static const struct file_operations osst_fops = {
 	.open =         os_scsi_tape_open,
 	.flush =        os_scsi_tape_flush,
 	.release =      os_scsi_tape_close,
+	.llseek =	noop_llseek,
 };
 
 static int osst_supports(struct scsi_device * SDp)
-- 
1.6.4.2


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

* [PATCH 10/12] st: Use noop_llseek() instead of default_llseek()
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (8 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 09/12] osst: Use noop_llseek() instead of default_llseek() Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 11/12] Do not fallback to default_llseek() when readdir() uses BKL Jan Blunck
  2010-04-09 14:00 ` [PATCH 12/12] Documentation/filesystems/Locking: Update documentation on llseek() wrt BKL Jan Blunck
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

st_open() suggests that llseek() doesn't work:
"We really want to do nonseekable_open(inode, filp); here, but some
versions of tar incorrectly call lseek on tapes and bail out if that
fails.  So we disallow pread() and pwrite(), but permit lseeks."

Instead of using the fallback default_llseek() the driver should use
noop_llseek() which leaves the file->f_pos untouched but succeeds.

Signed-off-by: Jan Blunck <jblunck@suse.de>
---
 drivers/scsi/st.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index f67d1a1..78e8f42 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -3961,6 +3961,7 @@ static const struct file_operations st_fops =
 	.open =		st_open,
 	.flush =	st_flush,
 	.release =	st_release,
+	.llseek =	noop_llseek,
 };
 
 static int st_probe(struct device *dev)
-- 
1.6.4.2


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

* [PATCH 11/12] Do not fallback to default_llseek() when readdir() uses BKL
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (9 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 10/12] st: " Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  2010-04-09 14:00 ` [PATCH 12/12] Documentation/filesystems/Locking: Update documentation on llseek() wrt BKL Jan Blunck
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox, Jan Blunck

Do not use the fallback default_llseek() if the readdir operation of the
filesystem still uses the big kernel lock. Since llseek() modifies
file->f_pos of the directory directly it may need locking to not confuse
readdir which usually uses file->f_pos directly as well. Since the special
characteristics of the BKL (unlocked on schedule) are not necessary in this
case, the inode mutex can be used for locking as provided by
generic_file_llseek(). This is only possible since all filesystems, except
reiserfs, either use a directory as a flat file or with disk address
offsets. Reiserfs on the other hand uses a 32bit hash off the filename as
the offset so generic_file_llseek() can get used as well since the hash is
always smaller than sb->s_maxbytes (= (512 << 32) - blocksize).

Signed-off-by: Jan Blunck <jblunck@suse.de>
Acked-by: Jan Kara <jack@suse.cz>
Acked-by: Anders Larsen <al@alarsen.net>
---
 fs/autofs/root.c          |    1 +
 fs/freevxfs/vxfs_lookup.c |    2 ++
 fs/isofs/dir.c            |    1 +
 fs/ncpfs/dir.c            |    1 +
 fs/qnx4/dir.c             |    1 +
 fs/reiserfs/dir.c         |    1 +
 fs/smbfs/dir.c            |    1 +
 fs/udf/dir.c              |    1 +
 8 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 4a1401c..d3aec77 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -30,6 +30,7 @@ const struct file_operations autofs_root_operations = {
 	.read		= generic_read_dir,
 	.readdir	= autofs_root_readdir,
 	.ioctl		= autofs_root_ioctl,
+	.llseek		= generic_file_llseek,
 };
 
 const struct inode_operations autofs_root_inode_operations = {
diff --git a/fs/freevxfs/vxfs_lookup.c b/fs/freevxfs/vxfs_lookup.c
index aee049c..0ec7bb2 100644
--- a/fs/freevxfs/vxfs_lookup.c
+++ b/fs/freevxfs/vxfs_lookup.c
@@ -57,6 +57,8 @@ const struct inode_operations vxfs_dir_inode_ops = {
 };
 
 const struct file_operations vxfs_dir_operations = {
+	.llseek =		generic_file_llseek,
+	.read =			generic_read_dir,
 	.readdir =		vxfs_readdir,
 };
 
diff --git a/fs/isofs/dir.c b/fs/isofs/dir.c
index 8ba5441..4816bd3 100644
--- a/fs/isofs/dir.c
+++ b/fs/isofs/dir.c
@@ -271,6 +271,7 @@ static int isofs_readdir(struct file *filp,
 
 const struct file_operations isofs_dir_operations =
 {
+	.llseek = generic_file_llseek,
 	.read = generic_read_dir,
 	.readdir = isofs_readdir,
 };
diff --git a/fs/ncpfs/dir.c b/fs/ncpfs/dir.c
index b8b5b30..c9cad9a 100644
--- a/fs/ncpfs/dir.c
+++ b/fs/ncpfs/dir.c
@@ -50,6 +50,7 @@ extern int ncp_symlink(struct inode *, struct dentry *, const char *);
 		      
 const struct file_operations ncp_dir_operations =
 {
+	.llseek		= generic_file_llseek,
 	.read		= generic_read_dir,
 	.readdir	= ncp_readdir,
 	.ioctl		= ncp_ioctl,
diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c
index 6f30c3d..3d3fd46 100644
--- a/fs/qnx4/dir.c
+++ b/fs/qnx4/dir.c
@@ -77,6 +77,7 @@ out:
 
 const struct file_operations qnx4_dir_operations =
 {
+	.llseek		= generic_file_llseek,
 	.read		= generic_read_dir,
 	.readdir	= qnx4_readdir,
 	.fsync		= simple_fsync,
diff --git a/fs/reiserfs/dir.c b/fs/reiserfs/dir.c
index c094f58..e1345fa 100644
--- a/fs/reiserfs/dir.c
+++ b/fs/reiserfs/dir.c
@@ -17,6 +17,7 @@ static int reiserfs_dir_fsync(struct file *filp, struct dentry *dentry,
 			      int datasync);
 
 const struct file_operations reiserfs_dir_operations = {
+	.llseek = generic_file_llseek,
 	.read = generic_read_dir,
 	.readdir = reiserfs_readdir,
 	.fsync = reiserfs_dir_fsync,
diff --git a/fs/smbfs/dir.c b/fs/smbfs/dir.c
index 3e4803b..cd11c54 100644
--- a/fs/smbfs/dir.c
+++ b/fs/smbfs/dir.c
@@ -37,6 +37,7 @@ static int smb_link(struct dentry *, struct inode *, struct dentry *);
 
 const struct file_operations smb_dir_operations =
 {
+	.llseek		= generic_file_llseek,
 	.read		= generic_read_dir,
 	.readdir	= smb_readdir,
 	.ioctl		= smb_ioctl,
diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index f0f2a43..e9f4ae0 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -207,6 +207,7 @@ static int udf_readdir(struct file *filp, void *dirent, filldir_t filldir)
 
 /* readdir and lookup functions */
 const struct file_operations udf_dir_operations = {
+	.llseek			= generic_file_llseek,
 	.read			= generic_read_dir,
 	.readdir		= udf_readdir,
 	.ioctl			= udf_ioctl,
-- 
1.6.4.2


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

* [PATCH 12/12] Documentation/filesystems/Locking: Update documentation on llseek() wrt BKL
  2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
                   ` (10 preceding siblings ...)
  2010-04-09 14:00 ` [PATCH 11/12] Do not fallback to default_llseek() when readdir() uses BKL Jan Blunck
@ 2010-04-09 14:00 ` Jan Blunck
  11 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-09 14:00 UTC (permalink / raw)
  To: Linux-Kernel Mailinglist
  Cc: Andrew Morton, Frederic Weisbecker, Arnd Bergmann, Alan Cox,
	Jan Blunck, Christoph Hellwig, John Kacur

The inode's i_size is not protected by the big kernel lock. Therefore it
does not make sense to recommend taking the BKL in filesystems llseek
operations. Instead it should use the inode's mutex or use just use
i_size_read() instead. Add a note that this is not protecting file->f_pos.

Signed-off-by: Jan Blunck <jblunck@suse.de>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: John Kacur <jkacur@redhat.com>
---
 Documentation/filesystems/Locking |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index 06bbbed..3cfe165 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -429,8 +429,9 @@ check_flags:		no
 implementations.  If your fs is not using generic_file_llseek, you
 need to acquire and release the appropriate locks in your ->llseek().
 For many filesystems, it is probably safe to acquire the inode
-semaphore.  Note some filesystems (i.e. remote ones) provide no
-protection for i_size so you will need to use the BKL.
+mutex or just to use i_size_read() instead.
+Note: this does not protect the file->f_pos against concurrent modifications
+since this is something the userspace has to take care about.
 
 Note: ext2_release() was *the* source of contention on fs-intensive
 loads and dropping BKL on ->release() helps to get rid of that (we still
-- 
1.6.4.2


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

* Re: [PATCH 04/12] frv: remove "struct file *" argument from sysctl  ->proc_handler
  2010-04-09 14:00 ` [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler Jan Blunck
@ 2010-04-09 16:23   ` Alexey Dobriyan
  2010-04-12  8:11     ` Jan Blunck
  0 siblings, 1 reply; 15+ messages in thread
From: Alexey Dobriyan @ 2010-04-09 16:23 UTC (permalink / raw)
  To: Jan Blunck
  Cc: Linux-Kernel Mailinglist, Andrew Morton, Frederic Weisbecker,
	Arnd Bergmann, Alan Cox

On Fri, Apr 9, 2010 at 5:00 PM, Jan Blunck <jblunck@suse.de> wrote:
> Seems that Alexey Dobriyan missed this usage of the file argument when
> removing it from ->proc_handler in commit 8d65af789f3e2cf4cfbdbf71a0f7a61ebcd41d38.

It wasn't missed, it was deliberately left out because I didn't understand
what's going on there.

I mean, sysctl handler touching ->f_pos, come on!

What's going on?

> --- a/arch/frv/kernel/sysctl.c
> +++ b/arch/frv/kernel/sysctl.c
> @@ -85,7 +86,7 @@ static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
>        }
>
>        /* read the state */
> -       if (filp->f_pos > 0) {
> +       if (*ppos > 0) {
>                *lenp = 0;
>                return 0;
>        }

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

* Re: [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler
  2010-04-09 16:23   ` Alexey Dobriyan
@ 2010-04-12  8:11     ` Jan Blunck
  0 siblings, 0 replies; 15+ messages in thread
From: Jan Blunck @ 2010-04-12  8:11 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: Linux-Kernel Mailinglist, Andrew Morton, Frederic Weisbecker,
	Arnd Bergmann, Alan Cox

On Fri, Apr 09, Alexey Dobriyan wrote:

> On Fri, Apr 9, 2010 at 5:00 PM, Jan Blunck <jblunck@suse.de> wrote:
> > Seems that Alexey Dobriyan missed this usage of the file argument when
> > removing it from ->proc_handler in commit 8d65af789f3e2cf4cfbdbf71a0f7a61ebcd41d38.
> 
> It wasn't missed, it was deliberately left out because I didn't understand
> what's going on there.
> 
> I mean, sysctl handler touching ->f_pos, come on!
> 
> What's going on?
> 

It is just a normal /proc/sys file. You can write a string to it to change the
some caching mode somehow.

Jan

> > --- a/arch/frv/kernel/sysctl.c
> > +++ b/arch/frv/kernel/sysctl.c
> > @@ -85,7 +86,7 @@ static int procctl_frv_cachemode(ctl_table *table, int write, struct file *filp,
> >        }
> >
> >        /* read the state */
> > -       if (filp->f_pos > 0) {
> > +       if (*ppos > 0) {
> >                *lenp = 0;
> >                return 0;
> >        }
Regards,
	Jan

-- 
Jan Blunck <jblunck@suse.de>

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

end of thread, other threads:[~2010-04-12  8:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-09 14:00 [PATCH 00/12] Multiple file->f_pos/nonseekable_open/llseek fixes (reposy) Jan Blunck
2010-04-09 14:00 ` [PATCH 01/12] osst: Update ppos instead of using file->f_pos Jan Blunck
2010-04-09 14:00 ` [PATCH 02/12] flash_read should update ppos instead of file->f_pos Jan Blunck
2010-04-09 14:00 ` [PATCH 03/12] eeprom_read()/eeprom_write() " Jan Blunck
2010-04-09 14:00 ` [PATCH 04/12] frv: remove "struct file *" argument from sysctl ->proc_handler Jan Blunck
2010-04-09 16:23   ` Alexey Dobriyan
2010-04-12  8:11     ` Jan Blunck
2010-04-09 14:00 ` [PATCH 05/12] mISDN: Remove unnecessary test on f_pos Jan Blunck
2010-04-09 14:00 ` [PATCH 06/12] zcrypt: Use nonseekable_open() Jan Blunck
2010-04-09 14:00 ` [PATCH 07/12] rtc-m41t80: " Jan Blunck
2010-04-09 14:00 ` [PATCH 08/12] Introduce noop_llseek() Jan Blunck
2010-04-09 14:00 ` [PATCH 09/12] osst: Use noop_llseek() instead of default_llseek() Jan Blunck
2010-04-09 14:00 ` [PATCH 10/12] st: " Jan Blunck
2010-04-09 14:00 ` [PATCH 11/12] Do not fallback to default_llseek() when readdir() uses BKL Jan Blunck
2010-04-09 14:00 ` [PATCH 12/12] Documentation/filesystems/Locking: Update documentation on llseek() wrt BKL Jan Blunck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.