Messages in this thread | | | From | OGAWA Hirofumi <> | Subject | Re: [PATCH 3/6] fat: BKL ioctl pushdown | Date | Thu, 06 May 2010 01:04:37 +0900 |
| |
John Kacur <jkacur@redhat.com> writes:
> Convert fat_generic_ioctl and fat_dir_ioctl to unlocked_ioctls > and push down the bkl into those functions.
I guess this is the part of batch ioctl conversion stuff though, those ioctl of FAT don't need BKL at all. Because all of those should already be protected by inode->i_mutex.
Removing BKL and then cleanup after this patch would be almost same with reverting this patch. So, could you just convert to unlocked_ioctl instead?
Thanks.
> Signed-off-by: John Kacur <jkacur@redhat.com> > --- > fs/fat/dir.c | 36 ++++++++++++++++++++++++------------ > fs/fat/fat.h | 3 +-- > fs/fat/file.c | 22 ++++++++++++++++------ > 3 files changed, 41 insertions(+), 20 deletions(-) > > diff --git a/fs/fat/dir.c b/fs/fat/dir.c > index 530b4ca..1b73e60 100644 > --- a/fs/fat/dir.c > +++ b/fs/fat/dir.c > @@ -19,6 +19,7 @@ > #include <linux/buffer_head.h> > #include <linux/compat.h> > #include <asm/uaccess.h> > +#include <linux/smp_lock.h> > #include "fat.h" > > /* > @@ -737,10 +738,11 @@ efault: \ > > FAT_IOCTL_FILLDIR_FUNC(fat_ioctl_filldir, __fat_dirent) > > -static int fat_ioctl_readdir(struct inode *inode, struct file *filp, > +static long fat_ioctl_readdir(struct file *filp, > void __user *dirent, filldir_t filldir, > int short_only, int both) > { > + struct inode *inode = filp->f_dentry->d_inode; > struct fat_ioctl_filldir_callback buf; > int ret; > > @@ -758,9 +760,10 @@ static int fat_ioctl_readdir(struct inode *inode, struct file *filp, > return ret; > } > > -static int fat_dir_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg) > +static long fat_dir_ioctl(struct file *filp, unsigned int cmd, > + unsigned long arg) > { > + long error; > struct __fat_dirent __user *d1 = (struct __fat_dirent __user *)arg; > int short_only, both; > > @@ -774,21 +777,31 @@ static int fat_dir_ioctl(struct inode *inode, struct file *filp, > both = 1; > break; > default: > - return fat_generic_ioctl(inode, filp, cmd, arg); > + lock_kernel(); > + error = fat_generic_ioctl(filp, cmd, arg); > + goto out; > } > > - if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_dirent[2]))) > - return -EFAULT; > + lock_kernel(); > + if (!access_ok(VERIFY_WRITE, d1, sizeof(struct __fat_dirent[2]))) { > + error = -EFAULT; > + goto out; > + } > /* > * Yes, we don't need this put_user() absolutely. However old > * code didn't return the right value. So, app use this value, > * in order to check whether it is EOF. > */ > - if (put_user(0, &d1->d_reclen)) > - return -EFAULT; > + if (put_user(0, &d1->d_reclen)) { > + error = -EFAULT; > + goto out; > + } > > - return fat_ioctl_readdir(inode, filp, d1, fat_ioctl_filldir, > + error = fat_ioctl_readdir(filp, d1, fat_ioctl_filldir, > short_only, both); > +out: > + unlock_kernel(); > + return error; > } > > #ifdef CONFIG_COMPAT > @@ -800,7 +813,6 @@ FAT_IOCTL_FILLDIR_FUNC(fat_compat_ioctl_filldir, compat_dirent) > static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd, > unsigned long arg) > { > - struct inode *inode = filp->f_path.dentry->d_inode; > struct compat_dirent __user *d1 = compat_ptr(arg); > int short_only, both; > > @@ -827,7 +839,7 @@ static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd, > if (put_user(0, &d1->d_reclen)) > return -EFAULT; > > - return fat_ioctl_readdir(inode, filp, d1, fat_compat_ioctl_filldir, > + return fat_ioctl_readdir(filp, d1, fat_compat_ioctl_filldir, > short_only, both); > } > #endif /* CONFIG_COMPAT */ > @@ -836,7 +848,7 @@ const struct file_operations fat_dir_operations = { > .llseek = generic_file_llseek, > .read = generic_read_dir, > .readdir = fat_readdir, > - .ioctl = fat_dir_ioctl, > + .unlocked_ioctl = fat_dir_ioctl, > #ifdef CONFIG_COMPAT > .compat_ioctl = fat_compat_dir_ioctl, > #endif > diff --git a/fs/fat/fat.h b/fs/fat/fat.h > index e6efdfa..23f9b2a 100644 > --- a/fs/fat/fat.h > +++ b/fs/fat/fat.h > @@ -298,8 +298,7 @@ extern int fat_free_clusters(struct inode *inode, int cluster); > extern int fat_count_free_clusters(struct super_block *sb); > > /* fat/file.c */ > -extern int fat_generic_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg); > +extern long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); > extern const struct file_operations fat_file_operations; > extern const struct inode_operations fat_file_inode_operations; > extern int fat_setattr(struct dentry * dentry, struct iattr * attr); > diff --git a/fs/fat/file.c b/fs/fat/file.c > index e8c159d..4f3044f 100644 > --- a/fs/fat/file.c > +++ b/fs/fat/file.c > @@ -16,6 +16,7 @@ > #include <linux/blkdev.h> > #include <linux/fsnotify.h> > #include <linux/security.h> > +#include <linux/smp_lock.h> > #include "fat.h" > > static int fat_ioctl_get_attributes(struct inode *inode, u32 __user *user_attr) > @@ -114,19 +115,28 @@ out: > return err; > } > > -int fat_generic_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg) > +long fat_generic_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > { > + long error; > + struct inode *inode = filp->f_dentry->d_inode; > u32 __user *user_attr = (u32 __user *)arg; > > + lock_kernel(); > + > switch (cmd) { > case FAT_IOCTL_GET_ATTRIBUTES: > - return fat_ioctl_get_attributes(inode, user_attr); > + error = fat_ioctl_get_attributes(inode, user_attr); > + break; > case FAT_IOCTL_SET_ATTRIBUTES: > - return fat_ioctl_set_attributes(filp, user_attr); > + error = fat_ioctl_set_attributes(filp, user_attr); > + break; > default: > - return -ENOTTY; /* Inappropriate ioctl for device */ > + error = -ENOTTY; /* Inappropriate ioctl for device */ > + break; > } > + > + unlock_kernel(); > + return error; > } > > static int fat_file_release(struct inode *inode, struct file *filp) > @@ -159,7 +169,7 @@ const struct file_operations fat_file_operations = { > .aio_write = generic_file_aio_write, > .mmap = generic_file_mmap, > .release = fat_file_release, > - .ioctl = fat_generic_ioctl, > + .unlocked_ioctl = fat_generic_ioctl, > .fsync = fat_file_fsync, > .splice_read = generic_file_splice_read, > };
-- OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
| |