Messages in this thread | | | Date | Wed, 9 Jul 2014 08:52:52 +0200 | Subject | Re: [PATCH] agp: remove read/write stubs | From | Mathias Krause <> |
| |
On 9 July 2014 00:48, Dave Airlie <airlied@gmail.com> wrote: > On 2 July 2014 16:15, Mathias Krause <minipli@googlemail.com> wrote: >> On 15 June 2014 23:02, Mathias Krause <minipli@googlemail.com> wrote: >>> The VFS layer handles those in the very same way, if unset. No need for >>> additional stubs. >>> >>> Signed-off-by: Mathias Krause <minipli@googlemail.com> >>> Cc: Alexander Viro <viro@zeniv.linux.org.uk> >>> --- >>> drivers/char/agp/frontend.c | 15 --------------- >>> 1 file changed, 15 deletions(-) >>> >>> diff --git a/drivers/char/agp/frontend.c b/drivers/char/agp/frontend.c >>> index b29703324e..09f17eb734 100644 >>> --- a/drivers/char/agp/frontend.c >>> +++ b/drivers/char/agp/frontend.c >>> @@ -710,19 +710,6 @@ static int agp_open(struct inode *inode, struct file *file) >>> return 0; >>> } >>> >>> - >>> -static ssize_t agp_read(struct file *file, char __user *buf, >>> - size_t count, loff_t * ppos) >>> -{ >>> - return -EINVAL; >>> -} >>> - >>> -static ssize_t agp_write(struct file *file, const char __user *buf, >>> - size_t count, loff_t * ppos) >>> -{ >>> - return -EINVAL; >>> -} >>> - >>> static int agpioc_info_wrap(struct agp_file_private *priv, void __user *arg) >>> { >>> struct agp_info userinfo; >>> @@ -1047,8 +1034,6 @@ static const struct file_operations agp_fops = >>> { >>> .owner = THIS_MODULE, >>> .llseek = no_llseek, >>> - .read = agp_read, >>> - .write = agp_write, >>> .unlocked_ioctl = agp_ioctl, >>> #ifdef CONFIG_COMPAT >>> .compat_ioctl = compat_agp_ioctl, >>> -- >>> 1.7.10.4 >>> >> >> Ping. Any objections, comments? David? > > Are you sure VFS handles this the same way? can you point me where it > checks ->read and returns -EINVAL, I'm probably just missing it in my > quick look over fs/read_write.c
The VFS code changed during the last merge window so the ->read and ->write tests are no longer directly visible. The presence of a ->read / ->write function is now remembered at open(2) time and stored in ->f_mode as a flag.
The corresponding commit for the VFS change is 7f7f25e82d "replace checking for ->read/->aio_read presence with check in ->f_mode". Despite its subject, it also handles ->write / ->aio_write. The flags are set in fs/open:do_dentry_open():
if ((f->f_mode & FMODE_READ) && likely(f->f_op->read || f->f_op->aio_read || f->f_op->read_iter)) f->f_mode |= FMODE_CAN_READ; if ((f->f_mode & FMODE_WRITE) && likely(f->f_op->write || f->f_op->aio_write || f->f_op->write_iter)) f->f_mode |= FMODE_CAN_WRITE;
With that change in mind, the redundancy of the agp_read() and agp_write() functions can be seen for read(2) in vfs_read():
if (!(file->f_mode & FMODE_CAN_READ)) return -EINVAL;
..and for write(2) in vfs_write():
if (!(file->f_mode & FMODE_CAN_WRITE)) return -EINVAL;
The other entry points (readv / writev / compat wrappers) have been changed accordingly, so those handle a missing ->read / ->write callback in the same way: returning -EINVAL.
Regards, Mathias
| |