* [PATCH] Push BKL into ->permission() calls
@ 2002-06-10 4:50 Paul Menage
0 siblings, 0 replies; 4+ messages in thread
From: Paul Menage @ 2002-06-10 4:50 UTC (permalink / raw)
To: viro, linux-fsdevel
Cc: trond.myklebust, jaharkes, braam, urban, linux-kernel, pmenage
The patch below (against 2.5.21) 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)
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.
Al, please forward this patch to Linus if you're happy with it.
Paul
diff -aur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.21/Documentation/filesystems/Locking linux-2.5.21-permission/Documentation/filesystems/Locking
--- linux-2.5.21/Documentation/filesystems/Locking Sat Jun 8 22:28:48 2002
+++ linux-2.5.21-permission/Documentation/filesystems/Locking Sun Jun 9 20:54:51 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 -aur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.21/fs/coda/dir.c linux-2.5.21-permission/fs/coda/dir.c
--- linux-2.5.21/fs/coda/dir.c Sat Jun 8 22:27:26 2002
+++ linux-2.5.21-permission/fs/coda/dir.c Sun Jun 9 20:31:24 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 -aur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.21/fs/intermezzo/dir.c linux-2.5.21-permission/fs/intermezzo/dir.c
--- linux-2.5.21/fs/intermezzo/dir.c Sat Jun 8 22:28:16 2002
+++ linux-2.5.21-permission/fs/intermezzo/dir.c Sun Jun 9 20:57:38 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 -aur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.21/fs/namei.c linux-2.5.21-permission/fs/namei.c
--- linux-2.5.21/fs/namei.c Sat Jun 8 22:28:50 2002
+++ linux-2.5.21-permission/fs/namei.c Sun Jun 9 20:25:31 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 -aur -X /mnt/elbrus/home/pmenage/dontdiff linux-2.5.21/fs/nfs/dir.c linux-2.5.21-permission/fs/nfs/dir.c
--- linux-2.5.21/fs/nfs/dir.c Sat Jun 8 22:29:55 2002
+++ linux-2.5.21-permission/fs/nfs/dir.c Sun Jun 9 20:30:36 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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Push BKL into ->permission() calls
2002-06-18 7:51 ` Andreas Dilger
@ 2002-06-18 8:27 ` Paul Menage
0 siblings, 0 replies; 4+ messages in thread
From: Paul Menage @ 2002-06-18 8:27 UTC (permalink / raw)
To: Andreas Dilger, viro
Cc: torvalds, linux-fsdevel, trond.myklebust, jaharkes, braam, urban,
linux-kernel, pmenage
>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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Push BKL into ->permission() calls
2002-06-18 7:27 Paul Menage
@ 2002-06-18 7:51 ` Andreas Dilger
2002-06-18 8:27 ` Paul Menage
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Dilger @ 2002-06-18 7:51 UTC (permalink / raw)
To: Paul Menage
Cc: viro, torvalds, linux-fsdevel, trond.myklebust, jaharkes, braam,
urban, linux-kernel
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/
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Push BKL into ->permission() calls
@ 2002-06-18 7:27 Paul Menage
2002-06-18 7:51 ` Andreas Dilger
0 siblings, 1 reply; 4+ messages in thread
From: Paul Menage @ 2002-06-18 7:27 UTC (permalink / raw)
To: viro, torvalds, linux-fsdevel
Cc: pmenage, trond.myklebust, jaharkes, braam, urban, linux-kernel
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2002-06-18 8:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-06-10 4:50 [PATCH] Push BKL into ->permission() calls Paul Menage
2002-06-18 7:27 Paul Menage
2002-06-18 7:51 ` Andreas Dilger
2002-06-18 8:27 ` Paul Menage
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.