Re: [RFC v2 2/5] Define new syscalls readv2,preadv2,writev2,pwritev2
On Fri, Sep 19, 2014 at 03:52:20AM -0700, Christoph Hellwig wrote:
> On Thu, Sep 18, 2014 at 11:48:23AM -0700, Darrick J. Wong wrote:
> > A few months ago I was working on extending these interfaces (well, the
> > p{read,write}* ones and AIO) to tack on an IO extension buffer at the end of
> > the syscall arguments.
>
> Honestly, that proposal is so but ugly that I treated it as an April
> first joke. I don't really think we want any of that overload mess.
I agree that a kitchen sink structure full of IO attributes is messy; at best
it avoids maintenance of horrifying parameter lists. The first two drafts of
the interface were too complicated and with the help of everyone who responded
to the first two threads with their criticisms, I've focused on paring down the
parts that people can screw up.
In v3, I define only a flat struct io_extension from which extensions can
copy_from_user whatever bits they want. Ideally I'd have three or four uses of
the extension API lined up for a more thoughtful design, but I'm just now
getting around to a second.
Clearly you have ideas of what constitutes good and bad API design. I've never
defined a major programming interface. Can you point me towards examples of
where we've gotten it right? Or possibly a discussion of design? The
materials from mkerrisk's 2007 talk about kernel API design seem to have gone
down with kernel.org, and I prefer to avoid badgering linux-api until I'm more
confident that I won't fall into the "this is apparently so bad that people
won't reply" trap.
I'm willing to learn, but snark about April Fool's jokes leading to silence
does not help me to improve the code or to help myself.
--D
--
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: [RFC v2 2/5] Define new syscalls readv2,preadv2,writev2,pwritev2
On Thu, Sep 18, 2014 at 11:48:23AM -0700, Darrick J. Wong wrote:
> A few months ago I was working on extending these interfaces (well, the
> p{read,write}* ones and AIO) to tack on an IO extension buffer at the end of
> the syscall arguments.
Honestly, that proposal is so but ugly that I treated it as an April
first joke. I don't really think we want any of that overload mess.
--
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: [RFC v2 2/5] Define new syscalls readv2,preadv2,writev2,pwritev2
On Wed, Sep 17, 2014 at 10:20:47PM +, Milosz Tanski wrote:
> New syscalls with an extra flag argument. For now all flags except for 0 are
> not supported.
>
> Signed-off-by: Milosz Tanski
> ---
> fs/read_write.c | 80
> +--
> include/linux/syscalls.h | 12 ++
> include/uapi/asm-generic/unistd.h | 10 -
> mm/filemap.c | 2 +-
> 4 files changed, 90 insertions(+), 14 deletions(-)
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 9f6d13d..3db2e87 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -864,6 +864,8 @@ ssize_t vfs_readv(struct file *file, const struct iovec
> __user *vec,
> return -EBADF;
> if (!(file->f_mode & FMODE_CAN_READ))
> return -EINVAL;
> + if (flags & ~0)
> + return -EINVAL;
>
> return do_readv_writev(READ, file, vec, vlen, pos, flags);
> }
> @@ -877,21 +879,23 @@ ssize_t vfs_writev(struct file *file, const struct
> iovec __user *vec,
> return -EBADF;
> if (!(file->f_mode & FMODE_CAN_WRITE))
> return -EINVAL;
> + if (flags & ~0)
> + return -EINVAL;
>
> return do_readv_writev(WRITE, file, vec, vlen, pos, flags);
> }
>
> EXPORT_SYMBOL(vfs_writev);
>
> -SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
> - unsigned long, vlen)
> +static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec,
> + unsigned long vlen, int flags)
> {
> struct fd f = fdget_pos(fd);
> ssize_t ret = -EBADF;
>
> if (f.file) {
> loff_t pos = file_pos_read(f.file);
> - ret = vfs_readv(f.file, vec, vlen, &pos, 0);
> + ret = vfs_readv(f.file, vec, vlen, &pos, flags);
> if (ret >= 0)
> file_pos_write(f.file, pos);
> fdput_pos(f);
> @@ -903,15 +907,15 @@ SYSCALL_DEFINE3(readv, unsigned long, fd, const struct
> iovec __user *, vec,
> return ret;
> }
>
> -SYSCALL_DEFINE3(writev, unsigned long, fd, const struct iovec __user *, vec,
> - unsigned long, vlen)
> +static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec,
> + unsigned long vlen, int flags)
> {
> struct fd f = fdget_pos(fd);
> ssize_t ret = -EBADF;
>
> if (f.file) {
> loff_t pos = file_pos_read(f.file);
> - ret = vfs_writev(f.file, vec, vlen, &pos, 0);
> + ret = vfs_writev(f.file, vec, vlen, &pos, flags);
> if (ret >= 0)
> file_pos_write(f.file, pos);
> fdput_pos(f);
> @@ -929,8 +933,9 @@ static inline loff_t pos_from_hilo(unsigned long high,
> unsigned long low)
> return (((loff_t)high << HALF_LONG_BITS) << HALF_LONG_BITS) | low;
> }
>
> -SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct iovec __user *, vec,
> - unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
> +static ssize_t do_preadv(unsigned long fd, const struct iovec __user *vec,
> + unsigned long vlen, unsigned long pos_l,
> + unsigned long pos_h, int flags)
> {
> loff_t pos = pos_from_hilo(pos_h, pos_l);
> struct fd f;
> @@ -943,7 +948,7 @@ SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct
> iovec __user *, vec,
> if (f.file) {
> ret = -ESPIPE;
> if (f.file->f_mode & FMODE_PREAD)
> - ret = vfs_readv(f.file, vec, vlen, &pos, 0);
> + ret = vfs_readv(f.file, vec, vlen, &pos, flags);
> fdput(f);
> }
>
> @@ -953,8 +958,9 @@ SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct
> iovec __user *, vec,
> return ret;
> }
>
> -SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct iovec __user *, vec,
> - unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
> +static ssize_t do_pwritev(unsigned long fd, const struct iovec __user *vec,
> + unsigned long vlen, unsigned long pos_l,
> + unsigned long pos_h, int flags)
> {
> loff_t pos = pos_from_hilo(pos_h, pos_l);
> struct fd f;
> @@ -967,7 +973,7 @@ SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct
> iovec __user *, vec,
> if (f.file) {
> ret = -ESPIPE;
> if (f.file->f_mode & FMODE_PWRITE)
> - ret = vfs_writev(f.file, vec, vlen, &pos, 0);
> + ret = vfs_writev(f.file, vec, vlen, &pos, flags);
> fdput(f);
> }
>
> @@ -977,6 +983,56 @@ SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct
> iovec __user *, vec,
> return ret;
> }
>
> +SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
> + unsigned long, vlen)
> +{
> + return do_readv(fd, vec, vle
[RFC v2 2/5] Define new syscalls readv2,preadv2,writev2,pwritev2
New syscalls with an extra flag argument. For now all flags except for 0 are
not supported.
Signed-off-by: Milosz Tanski
---
fs/read_write.c | 80 +--
include/linux/syscalls.h | 12 ++
include/uapi/asm-generic/unistd.h | 10 -
mm/filemap.c | 2 +-
4 files changed, 90 insertions(+), 14 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 9f6d13d..3db2e87 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -864,6 +864,8 @@ ssize_t vfs_readv(struct file *file, const struct iovec
__user *vec,
return -EBADF;
if (!(file->f_mode & FMODE_CAN_READ))
return -EINVAL;
+ if (flags & ~0)
+ return -EINVAL;
return do_readv_writev(READ, file, vec, vlen, pos, flags);
}
@@ -877,21 +879,23 @@ ssize_t vfs_writev(struct file *file, const struct iovec
__user *vec,
return -EBADF;
if (!(file->f_mode & FMODE_CAN_WRITE))
return -EINVAL;
+ if (flags & ~0)
+ return -EINVAL;
return do_readv_writev(WRITE, file, vec, vlen, pos, flags);
}
EXPORT_SYMBOL(vfs_writev);
-SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
- unsigned long, vlen)
+static ssize_t do_readv(unsigned long fd, const struct iovec __user *vec,
+ unsigned long vlen, int flags)
{
struct fd f = fdget_pos(fd);
ssize_t ret = -EBADF;
if (f.file) {
loff_t pos = file_pos_read(f.file);
- ret = vfs_readv(f.file, vec, vlen, &pos, 0);
+ ret = vfs_readv(f.file, vec, vlen, &pos, flags);
if (ret >= 0)
file_pos_write(f.file, pos);
fdput_pos(f);
@@ -903,15 +907,15 @@ SYSCALL_DEFINE3(readv, unsigned long, fd, const struct
iovec __user *, vec,
return ret;
}
-SYSCALL_DEFINE3(writev, unsigned long, fd, const struct iovec __user *, vec,
- unsigned long, vlen)
+static ssize_t do_writev(unsigned long fd, const struct iovec __user *vec,
+unsigned long vlen, int flags)
{
struct fd f = fdget_pos(fd);
ssize_t ret = -EBADF;
if (f.file) {
loff_t pos = file_pos_read(f.file);
- ret = vfs_writev(f.file, vec, vlen, &pos, 0);
+ ret = vfs_writev(f.file, vec, vlen, &pos, flags);
if (ret >= 0)
file_pos_write(f.file, pos);
fdput_pos(f);
@@ -929,8 +933,9 @@ static inline loff_t pos_from_hilo(unsigned long high,
unsigned long low)
return (((loff_t)high << HALF_LONG_BITS) << HALF_LONG_BITS) | low;
}
-SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct iovec __user *, vec,
- unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
+static ssize_t do_preadv(unsigned long fd, const struct iovec __user *vec,
+unsigned long vlen, unsigned long pos_l,
+unsigned long pos_h, int flags)
{
loff_t pos = pos_from_hilo(pos_h, pos_l);
struct fd f;
@@ -943,7 +948,7 @@ SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct
iovec __user *, vec,
if (f.file) {
ret = -ESPIPE;
if (f.file->f_mode & FMODE_PREAD)
- ret = vfs_readv(f.file, vec, vlen, &pos, 0);
+ ret = vfs_readv(f.file, vec, vlen, &pos, flags);
fdput(f);
}
@@ -953,8 +958,9 @@ SYSCALL_DEFINE5(preadv, unsigned long, fd, const struct
iovec __user *, vec,
return ret;
}
-SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct iovec __user *, vec,
- unsigned long, vlen, unsigned long, pos_l, unsigned long, pos_h)
+static ssize_t do_pwritev(unsigned long fd, const struct iovec __user *vec,
+ unsigned long vlen, unsigned long pos_l,
+ unsigned long pos_h, int flags)
{
loff_t pos = pos_from_hilo(pos_h, pos_l);
struct fd f;
@@ -967,7 +973,7 @@ SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct
iovec __user *, vec,
if (f.file) {
ret = -ESPIPE;
if (f.file->f_mode & FMODE_PWRITE)
- ret = vfs_writev(f.file, vec, vlen, &pos, 0);
+ ret = vfs_writev(f.file, vec, vlen, &pos, flags);
fdput(f);
}
@@ -977,6 +983,56 @@ SYSCALL_DEFINE5(pwritev, unsigned long, fd, const struct
iovec __user *, vec,
return ret;
}
+SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
+ unsigned long, vlen)
+{
+ return do_readv(fd, vec, vlen, 0);
+}
+
+SYSCALL_DEFINE4(readv2, unsigned long, fd, const struct iovec __user *, vec,
+ unsigned long, vlen, int, flags)
+{
+ return do_readv(fd, vec, vlen, flags);
+}
+
