[PATCH] Push BKL into ->permission() calls

[PATCH] Push BKL into ->permission() calls

[Paul Menage]

This patch (against 2.5.22) removes the BKL from around the call 
to i_op->permission() in fs/namei.c, and pushes the BKL into those 
filesystems that have permission() methods that require it:

fs/nfs/dir.c:nfs_permission():  Take BKL if we need to do RPC

fs/coda/dir.c:coda_permission(): Take BKL if mask != 0

fs/coda/pioctl.c:coda_ioctl_permission(): BKL not needed

net/wanrouter/wanproc.c:router_proc_perms(): BKL not needed 

fs/intermezzo/dir.c:presto_permission(): Always take BKL (and avoid
abuse of inode method table) (but it still doesn't compile due to 
earlier breakage)

fs/proc/base.c:proc_permission(): BKL not needed

fs/smbfs/file.c:smb_file_permission(): BKL not needed

kernel/sysctl.c:proc_sys_permission(): BKL not needed

Out-of-tree filesystems that have their own permission() method will 
need updating if they're relying on the BKL.

All feedback welcome.

Paul

diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/Documentation/filesystems/Locking linux-2.5.22-permission/Documentation/filesystems/Locking
--- linux-2.5.22/Documentation/filesystems/Locking	Sat Jun  8 22:28:48 2002
+++ linux-2.5.22-permission/Documentation/filesystems/Locking	Mon Jun 17 16:34:09 2002
@@ -50,27 +50,27 @@
 	int (*removexattr) (struct dentry *, const char *);
 
 locking rules:
-	all may block
-		BKL	i_sem(inode)
-lookup:		no	yes
-create:		no	yes
-link:		no	yes (both)
-mknod:		no	yes
-symlink:	no	yes
-mkdir:		no	yes
-unlink:		no	yes (both)
-rmdir:		no	yes (both)	(see below)
-rename:		no	yes (all)	(see below)
-readlink:	no	no
-follow_link:	no	no
-truncate:	no	yes		(see below)
-setattr:	no	yes
-permission:	yes	no
-getattr:	no	no
-setxattr:	no	yes
-getxattr:	no	yes
-listxattr:	no	yes
-removexattr:	no	yes
+	all may block, none have BKL
+		i_sem(inode)
+lookup:		yes
+create:		yes
+link:		yes (both)
+mknod:		yes
+symlink:	yes
+mkdir:		yes
+unlink:		yes (both)
+rmdir:		yes (both)	(see below)
+rename:		yes (all)	(see below)
+readlink:	no
+follow_link:	no
+truncate:	yes		(see below)
+setattr:	yes
+permission:	no
+getattr:	no
+setxattr:	yes
+getxattr:	yes
+listxattr:	yes
+removexattr:	yes
 	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_sem on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/coda/dir.c linux-2.5.22-permission/fs/coda/dir.c
--- linux-2.5.22/fs/coda/dir.c	Sat Jun  8 22:27:26 2002
+++ linux-2.5.22-permission/fs/coda/dir.c	Mon Jun 17 16:34:09 2002
@@ -147,21 +147,26 @@
 
 int coda_permission(struct inode *inode, int mask)
 {
-        int error;
+        int error = 0;
  
 	if (!mask)
 		return 0; 
 
+	lock_kernel();
+
 	coda_vfs_stat.permission++;
 
 	if (coda_cache_check(inode, mask))
-		return 0; 
+		goto out; 
 
         error = venus_access(inode->i_sb, coda_i2f(inode), mask);
     
 	if (!error)
 		coda_cache_enter(inode, mask);
 
+ out:
+	unlock_kernel();
+
         return error; 
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/intermezzo/dir.c linux-2.5.22-permission/fs/intermezzo/dir.c
--- linux-2.5.22/fs/intermezzo/dir.c	Sat Jun  8 22:28:16 2002
+++ linux-2.5.22-permission/fs/intermezzo/dir.c	Mon Jun 17 16:34:09 2002
@@ -785,13 +785,15 @@
 {
         unsigned short mode = inode->i_mode;
         struct presto_cache *cache;
-        int rc;
+        int rc = 0;
 
+	lock_kernel();
         ENTRY;
+
         if ( presto_can_ilookup() && !(mask & S_IWOTH)) {
                 CDEBUG(D_CACHE, "ilookup on %ld OK\n", inode->i_ino);
-                EXIT;
-                return 0;
+		EXIT;
+		goto out;
         }
 
         cache = presto_get_cache(inode);
@@ -803,25 +805,22 @@
 
                 if ( S_ISREG(mode) && fiops && fiops->permission ) {
                         EXIT;
-                        return fiops->permission(inode, mask);
+                        rc = fiops->permission(inode, mask);
+			goto out;
                 }
                 if ( S_ISDIR(mode) && diops && diops->permission ) {
                         EXIT;
-                        return diops->permission(inode, mask);
+                        rc = diops->permission(inode, mask);
+			goto out;
                 }
         }
 
-        /* The cache filesystem doesn't have its own permission function,
-         * but we don't want to duplicate the VFS code here.  In order
-         * to avoid looping from permission calling this function again,
-         * we temporarily override the permission operation while we call
-         * the VFS permission function.
-         */
-        inode->i_op->permission = NULL;
-        rc = permission(inode, mask);
-        inode->i_op->permission = &presto_permission;
+        rc = vfs_permission(inode, mask);
 
         EXIT;
+
+ out:
+	unlock_kernel();
         return rc;
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/namei.c linux-2.5.22-permission/fs/namei.c
--- linux-2.5.22/fs/namei.c	Sat Jun  8 22:28:50 2002
+++ linux-2.5.22-permission/fs/namei.c	Mon Jun 17 16:34:09 2002
@@ -204,13 +204,8 @@
 
 int permission(struct inode * inode,int mask)
 {
-	if (inode->i_op && inode->i_op->permission) {
-		int retval;
-		lock_kernel();
-		retval = inode->i_op->permission(inode, mask);
-		unlock_kernel();
-		return retval;
-	}
+	if (inode->i_op && inode->i_op->permission)
+		return inode->i_op->permission(inode, mask);
 	return vfs_permission(inode, mask);
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/nfs/dir.c linux-2.5.22-permission/fs/nfs/dir.c
--- linux-2.5.22/fs/nfs/dir.c	Sat Jun  8 22:29:55 2002
+++ linux-2.5.22-permission/fs/nfs/dir.c	Mon Jun 17 16:34:09 2002
@@ -1123,6 +1123,8 @@
 	    && error != -EACCES)
 		goto out;
 
+	lock_kernel();
+
 	error = NFS_PROTO(inode)->access(inode, mask, 0);
 
 	if (error == -EACCES && NFS_CLIENT(inode)->cl_droppriv &&
@@ -1130,6 +1132,8 @@
 	    (current->fsuid != current->uid || current->fsgid != current->gid))
 		error = NFS_PROTO(inode)->access(inode, mask, 1);
 
+	unlock_kernel();
+
  out:
 	return error;
 }

Re: [PATCH] Push BKL into ->permission() calls

[Andreas Dilger]

On Jun 18, 2002  00:27 -0700, Paul Menage wrote:
> This patch (against 2.5.22) removes the BKL from around the call 
> to i_op->permission() in fs/namei.c, and pushes the BKL into those 
> filesystems that have permission() methods that require it:
>
> Out-of-tree filesystems that have their own permission() method will 
> need updating if they're relying on the BKL.

Please update Documentation/filesystems/porting with this info as part
of your patch.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/

Re: [PATCH] Push BKL into ->permission() calls

[Paul Menage]

>On Jun 18, 2002  00:27 -0700, Paul Menage wrote:
>Please update Documentation/filesystems/porting with this info as part
>of your patch.
>

OK. Documentation/filesystems/porting is included this time.

==============================================================

This patch (against 2.5.22) removes the BKL from around the call 
to i_op->permission() in fs/namei.c, and pushes the BKL into those 
filesystems that have permission() methods that require it:

fs/nfs/dir.c:nfs_permission():  Take BKL if we need to do RPC

fs/coda/dir.c:coda_permission(): Take BKL if mask != 0

fs/coda/pioctl.c:coda_ioctl_permission(): BKL not needed

net/wanrouter/wanproc.c:router_proc_perms(): BKL not needed 

fs/intermezzo/dir.c:presto_permission(): Always take BKL (and avoid
abuse of inode method table) (but it still doesn't compile due to 
earlier breakage)

fs/proc/base.c:proc_permission(): BKL not needed

fs/smbfs/file.c:smb_file_permission(): BKL not needed

kernel/sysctl.c:proc_sys_permission(): BKL not needed

Out-of-tree filesystems that have their own permission() method will 
need updating if they're relying on the BKL.

All feedback welcome.

Paul

diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/Documentation/filesystems/Locking linux-2.5.22-permission/Documentation/filesystems/Locking
--- linux-2.5.22/Documentation/filesystems/Locking	Sat Jun  8 22:28:48 2002
+++ linux-2.5.22-permission/Documentation/filesystems/Locking	Mon Jun 17 16:34:09 2002
@@ -50,27 +50,27 @@
 	int (*removexattr) (struct dentry *, const char *);
 
 locking rules:
-	all may block
-		BKL	i_sem(inode)
-lookup:		no	yes
-create:		no	yes
-link:		no	yes (both)
-mknod:		no	yes
-symlink:	no	yes
-mkdir:		no	yes
-unlink:		no	yes (both)
-rmdir:		no	yes (both)	(see below)
-rename:		no	yes (all)	(see below)
-readlink:	no	no
-follow_link:	no	no
-truncate:	no	yes		(see below)
-setattr:	no	yes
-permission:	yes	no
-getattr:	no	no
-setxattr:	no	yes
-getxattr:	no	yes
-listxattr:	no	yes
-removexattr:	no	yes
+	all may block, none have BKL
+		i_sem(inode)
+lookup:		yes
+create:		yes
+link:		yes (both)
+mknod:		yes
+symlink:	yes
+mkdir:		yes
+unlink:		yes (both)
+rmdir:		yes (both)	(see below)
+rename:		yes (all)	(see below)
+readlink:	no
+follow_link:	no
+truncate:	yes		(see below)
+setattr:	yes
+permission:	no
+getattr:	no
+setxattr:	yes
+getxattr:	yes
+listxattr:	yes
+removexattr:	yes
 	Additionally, ->rmdir(), ->unlink() and ->rename() have ->i_sem on
 victim.
 	cross-directory ->rename() has (per-superblock) ->s_vfs_rename_sem.
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/coda/dir.c linux-2.5.22-permission/fs/coda/dir.c
--- linux-2.5.22/fs/coda/dir.c	Sat Jun  8 22:27:26 2002
+++ linux-2.5.22-permission/fs/coda/dir.c	Mon Jun 17 16:34:09 2002
@@ -147,21 +147,26 @@
 
 int coda_permission(struct inode *inode, int mask)
 {
-        int error;
+        int error = 0;
  
 	if (!mask)
 		return 0; 
 
+	lock_kernel();
+
 	coda_vfs_stat.permission++;
 
 	if (coda_cache_check(inode, mask))
-		return 0; 
+		goto out; 
 
         error = venus_access(inode->i_sb, coda_i2f(inode), mask);
     
 	if (!error)
 		coda_cache_enter(inode, mask);
 
+ out:
+	unlock_kernel();
+
         return error; 
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/intermezzo/dir.c linux-2.5.22-permission/fs/intermezzo/dir.c
--- linux-2.5.22/fs/intermezzo/dir.c	Sat Jun  8 22:28:16 2002
+++ linux-2.5.22-permission/fs/intermezzo/dir.c	Mon Jun 17 16:34:09 2002
@@ -785,13 +785,15 @@
 {
         unsigned short mode = inode->i_mode;
         struct presto_cache *cache;
-        int rc;
+        int rc = 0;
 
+	lock_kernel();
         ENTRY;
+
         if ( presto_can_ilookup() && !(mask & S_IWOTH)) {
                 CDEBUG(D_CACHE, "ilookup on %ld OK\n", inode->i_ino);
-                EXIT;
-                return 0;
+		EXIT;
+		goto out;
         }
 
         cache = presto_get_cache(inode);
@@ -803,25 +805,22 @@
 
                 if ( S_ISREG(mode) && fiops && fiops->permission ) {
                         EXIT;
-                        return fiops->permission(inode, mask);
+                        rc = fiops->permission(inode, mask);
+			goto out;
                 }
                 if ( S_ISDIR(mode) && diops && diops->permission ) {
                         EXIT;
-                        return diops->permission(inode, mask);
+                        rc = diops->permission(inode, mask);
+			goto out;
                 }
         }
 
-        /* The cache filesystem doesn't have its own permission function,
-         * but we don't want to duplicate the VFS code here.  In order
-         * to avoid looping from permission calling this function again,
-         * we temporarily override the permission operation while we call
-         * the VFS permission function.
-         */
-        inode->i_op->permission = NULL;
-        rc = permission(inode, mask);
-        inode->i_op->permission = &presto_permission;
+        rc = vfs_permission(inode, mask);
 
         EXIT;
+
+ out:
+	unlock_kernel();
         return rc;
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/namei.c linux-2.5.22-permission/fs/namei.c
--- linux-2.5.22/fs/namei.c	Sat Jun  8 22:28:50 2002
+++ linux-2.5.22-permission/fs/namei.c	Mon Jun 17 16:34:09 2002
@@ -204,13 +204,8 @@
 
 int permission(struct inode * inode,int mask)
 {
-	if (inode->i_op && inode->i_op->permission) {
-		int retval;
-		lock_kernel();
-		retval = inode->i_op->permission(inode, mask);
-		unlock_kernel();
-		return retval;
-	}
+	if (inode->i_op && inode->i_op->permission)
+		return inode->i_op->permission(inode, mask);
 	return vfs_permission(inode, mask);
 }
 
diff -X /mnt/elbrus/home/pmenage/dontdiff -Naur linux-2.5.22/fs/nfs/dir.c linux-2.5.22-permission/fs/nfs/dir.c
--- linux-2.5.22/fs/nfs/dir.c	Sat Jun  8 22:29:55 2002
+++ linux-2.5.22-permission/fs/nfs/dir.c	Mon Jun 17 16:34:09 2002
@@ -1123,6 +1123,8 @@
 	    && error != -EACCES)
 		goto out;
 
+	lock_kernel();
+
 	error = NFS_PROTO(inode)->access(inode, mask, 0);
 
 	if (error == -EACCES && NFS_CLIENT(inode)->cl_droppriv &&
@@ -1130,6 +1132,8 @@
 	    (current->fsuid != current->uid || current->fsgid != current->gid))
 		error = NFS_PROTO(inode)->access(inode, mask, 1);
 
+	unlock_kernel();
+
  out:
 	return error;
 }
--- linux-2.5.22/Documentation/filesystems/porting	Mon Jun 17 16:24:54 2002
+++ linux-2.5.22-permission/Documentation/filesystems/porting	Tue Jun 18 01:16:07 2002
@@ -81,9 +81,9 @@
 [mandatory]
 
 ->lookup(), ->truncate(), ->create(), ->unlink(), ->mknod(), ->mkdir(),
-->rmdir(), ->link(), ->lseek(), ->symlink(), ->rename() and ->readdir() 
-are called without BKL now.  Grab it on the entry, drop upon return - that 
-will guarantee the same locking you used to have.  If your method or its
+->rmdir(), ->link(), ->lseek(), ->symlink(), ->rename(), ->permission()
+and ->readdir() are called without BKL now.  Grab it on entry, drop upon return
+- that will guarantee the same locking you used to have.  If your method or its
 parts do not need BKL - better yet, now you can shift lock_kernel() and
 unlock_kernel() so that they would protect exactly what needs to be
 protected.

2.5.22 problems with compile.h

[Hans E. Kristiansen]

I really need some help with compiling version 2.5.22

>From a clean install, I can compile, but I get an error with compile.h (Do
not know how to make compile.h). If I compile again, I get a working kernel
(bzImage), "depmod -ae -F xx " works like a charm. But, when I reboot with
the new kernel, I can not load any modules. None, they all have symbol
problems.

I use grub for loading, and I have verified that I have the correct
System.map

My last kernel is 2.5.19, which works like a charm ( well... ).

To compile a kernel, I use "make dep clean bzImage modules modules_install",
except for the first compile after an upgrade where I run "make oldconfig"
first.

Thanks,
Hans E.

Re: 2.5.22 problems with compile.h

[Martin Diehl]

On Tue, 18 Jun 2002, Hans E. Kristiansen wrote:

> I really need some help with compiling version 2.5.22
> 
> >From a clean install, I can compile, but I get an error with compile.h (Do
> not know how to make compile.h). If I compile again, I get a working kernel

same here - see below.

> (bzImage), "depmod -ae -F xx " works like a charm. But, when I reboot with
> the new kernel, I can not load any modules. None, they all have symbol
> problems.

There are two patches from Kai Germaschewski in the "2.5.22 broke
modversions" thread which solved this issue for me.

> To compile a kernel, I use "make dep clean bzImage modules modules_install",

replacing this by separate "make dep; make clean; make bzImage; make
modules; make modules_install" invokations solved the compile.h problem
for me.

HTH
Martin

Re: 2.5.22 problems with compile.h

[Kai Germaschewski]

On Tue, 18 Jun 2002, Hans E. Kristiansen wrote:

> >From a clean install, I can compile, but I get an error with compile.h (Do
> not know how to make compile.h). If I compile again, I get a working kernel
> (bzImage), "depmod -ae -F xx " works like a charm. But, when I reboot with
> the new kernel, I can not load any modules. None, they all have symbol
> problems.

Seems that I screwed up that version a bit ;( 

For now, don't use something like "make dep clean bzImage", but rather
"make clean; make dep bzImage" - it'll work again in the next release,
though.

Modversions also have bugs, though I can't see how they lead to the effect 
you describe. Anyway, try changing CONFIG_MODVERSIONS to off for now,
again, it should work again in the next release.

Let me know of further problems ;)

--Kai

Re: 2.5.22 problems with compile.h

[Hans E. Kristiansen]

I applied your patch which was included in an earlier email on the advice
from Mr. Martin Deal, and it works! Can't ask for more. At last I can go
back to my userland programming.

Perhaps the real problem is that Linus released 2.5.22 on a Monday :)

TTFN,
Hans E.

----- Original Message -----
From: "Kai Germaschewski" <kai@tp1.ruhr-uni-bochum.de>
To: "Hans E. Kristiansen" <hans@tropic.net>
Cc: <linux-kernel@vger.kernel.org>
Sent: Tuesday, June 18, 2002 22:36
Subject: Re: 2.5.22 problems with compile.h


> On Tue, 18 Jun 2002, Hans E. Kristiansen wrote:
>
> > >From a clean install, I can compile, but I get an error with compile.h
(Do
> > not know how to make compile.h). If I compile again, I get a working
kernel
> > (bzImage), "depmod -ae -F xx " works like a charm. But, when I reboot
with

--- snip ----