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

From: Paul Menage (pmenage@ensim.com)
Date: Tue Jun 18 2002 - 03:27:38 EST


>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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Jun 23 2002 - 22:00:15 EST