Re: [PATCH] agp: remove read/write stubs
> > > > Ping. Any more comments, David? > > Ping, ping. David, did you had time to look at the patch again? Merged finally thanks for reminding me, agp is kinda a dead zone. Dave. -- 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/
Re: [PATCH] agp: remove read/write stubs
On 22 July 2014 21:50, Mathias Krause wrote: > On 9 July 2014 08:52, Mathias Krause wrote: >> On 9 July 2014 00:48, Dave Airlie wrote: >>> On 2 July 2014 16:15, Mathias Krause wrote: On 15 June 2014 23:02, Mathias Krause wrote: > The VFS layer handles those in the very same way, if unset. No need for > additional stubs. > > Signed-off-by: Mathias Krause > Cc: Alexander Viro > --- > 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/
Re: [PATCH] agp: remove read/write stubs
On 9 July 2014 08:52, Mathias Krause wrote: > On 9 July 2014 00:48, Dave Airlie wrote: >> On 2 July 2014 16:15, Mathias Krause wrote: >>> On 15 June 2014 23:02, Mathias Krause wrote: The VFS layer handles those in the very same way, if unset. No need for additional stubs. Signed-off-by: Mathias Krause Cc: Alexander Viro --- 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? -- 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/
Re: [PATCH] agp: remove read/write stubs
On 9 July 2014 00:48, Dave Airlie wrote: > On 2 July 2014 16:15, Mathias Krause wrote: >> On 15 June 2014 23:02, Mathias Krause wrote: >>> The VFS layer handles those in the very same way, if unset. No need for >>> additional stubs. >>> >>> Signed-off-by: Mathias Krause >>> Cc: Alexander Viro >>> --- >>> 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 -- 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/
Re: [PATCH] agp: remove read/write stubs
On 2 July 2014 16:15, Mathias Krause wrote: > On 15 June 2014 23:02, Mathias Krause wrote: >> The VFS layer handles those in the very same way, if unset. No need for >> additional stubs. >> >> Signed-off-by: Mathias Krause >> Cc: Alexander Viro >> --- >> 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 Dave. -- 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/
Re: [PATCH] agp: remove read/write stubs
On 15 June 2014 23:02, Mathias Krause wrote: > The VFS layer handles those in the very same way, if unset. No need for > additional stubs. > > Signed-off-by: Mathias Krause > Cc: Alexander Viro > --- > 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? -- 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/