On 22 July 2014 21:50, Mathias Krause <mini...@googlemail.com> wrote: > On 9 July 2014 08:52, Mathias Krause <mini...@googlemail.com> wrote: >> On 9 July 2014 00:48, Dave Airlie <airl...@gmail.com> wrote: >>> On 2 July 2014 16:15, Mathias Krause <mini...@googlemail.com> wrote: >>>> On 15 June 2014 23:02, Mathias Krause <mini...@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 <mini...@googlemail.com> >>>>> Cc: Alexander Viro <v...@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 > > Ping. Any more comments, David?
Ping, ping. David, did you had time to look at the patch again? Regards, Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/