2009-04-02 06:21:52

by Suresh Jayaraman

[permalink] [raw]
Subject: [PATCH] nfs: add support for splice writes

This patch attempts to add splice writes support. In essence, it just
calls generic_file_splice_write() after doing a little sanity check.
This would allow LTTng users that are using NFS to store trace data.
There could be more applications that could be benefitted too.

I have tested this using the Jens' test program and have found no
real issues. The test program is inlined below:

/*
* splice-out.c: Splice stdout to file
*/
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>

#define SPLICE_SIZE (64*1024)

int main(int argc, char *argv[])
{
int fd;

if (argc < 2) {
printf("%s: outfile\n", argv[0]);
return 1;
}

fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC, 0644);
if (fd < 0) {
perror("open");
return 1;
}

do {
int ret = splice(STDIN_FILENO, NULL, fd, NULL, SPLICE_SIZE, 0);

if (ret < 0) {
perror("splice");
break;
} else if (ret < SPLICE_SIZE)
break;
} while (1);

close(fd);
return 0;
}

Compile with -D _GNU_SOURCE and do something like:
echo "some stuff" | ./splice-out <outfile>

Signed-off-by: Suresh Jayaraman <[email protected]>
---
fs/nfs/file.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 90f292b..13d6a00 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
size_t count, unsigned int flags);
static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
unsigned long nr_segs, loff_t pos);
+static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
+ struct file *filp, loff_t *ppos,
+ size_t count, unsigned int flags);
static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
unsigned long nr_segs, loff_t pos);
static int nfs_file_flush(struct file *, fl_owner_t id);
@@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
.lock = nfs_lock,
.flock = nfs_flock,
.splice_read = nfs_file_splice_read,
+ .splice_write = nfs_file_splice_write,
.check_flags = nfs_check_flags,
.setlease = nfs_setlease,
};
@@ -550,6 +554,26 @@ out_swapfile:
goto out;
}

+static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
+ struct file *filp, loff_t *ppos,
+ size_t count, unsigned int flags)
+{
+ struct dentry *dentry = filp->f_path.dentry;
+ struct inode *inode = dentry->d_inode;
+
+ dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
+ dentry->d_parent->d_name.name, dentry->d_name.name,
+ (unsigned long) count, (unsigned long long) *ppos);
+
+ if (IS_SWAPFILE(inode)) {
+ printk(KERN_INFO "NFS: attempt to write to active swap"
+ "file!\n");
+ return -EBUSY;
+ }
+
+ return generic_file_splice_write(pipe, filp, ppos, count, flags);
+}
+
static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
{
struct inode *inode = filp->f_mapping->host;



2009-04-20 05:39:15

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [ltt-dev] [PATCH] nfs: add support for splice writes

Hi Trond,

Do you think this patch is OK? Can this be considered for merging?

Thanks,

Masahiro Tamori wrote:
> Hello Suresh and Mathieu,
>
> 2009/4/2 Suresh Jayaraman <[email protected]>:
>> Mathieu Desnoyers wrote:
>>> * Suresh Jayaraman ([email protected]) wrote:
>>>> This patch attempts to add splice writes support. In essence, it just
>>>> calls generic_file_splice_write() after doing a little sanity check.
>>>> This would allow LTTng users that are using NFS to store trace data.
>>>> There could be more applications that could be benefitted too.
>>>>
>>>> I have tested this using the Jens' test program and have found no
>>>> real issues. The test program is inlined below:
>>>>
>>> There is just a small checkpatch nit that I'll fix directly in place in
>>> the LTTng tree.
>>>
>>> WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
>>> #93: FILE: fs/nfs/file.c:564:
>>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>>>
>>> total: 0 errors, 1 warnings, 42 lines checked
>>>
>> Yes, I noticed it. There are quite a few places in nfs code where we
>> happened to use that (that doesn't imply that it shouldn't be fixed), so
>> I thought it's OK.
>>
>>> That's great ! I'll pull it in the LTTng tree so my users can try it.
>>> They currently cannot write LTTng traces to NFS mounts anyway.
>> Yes, please report the test results. Would appreciate it very much.
>>
>>
>> Thanks,
>
> Thank you for creating a patch !!
>
> I tested it, and it works fine.
>
> My environment is following,
> LTTV 0.12.10
> LTTng 0.100
> LTT Control 0.65
> Kernles 2.6.29
>
> LTTng version is old now, but the test is passed on ARM11 target.
>
> Furthermore, I run the splice test and the test is passed too.
> The test code is copied from
> http://kzk9.net/blog/2007/05/splice2.html
> (Japanese page)
>
> Thanks all people who commented this issue.
>
> Best regards,
> Masahiro Tamori
>
>>>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>>>> index 90f292b..13d6a00 100644
>>>> --- a/fs/nfs/file.c
>>>> +++ b/fs/nfs/file.c
>>>> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
>>>> size_t count, unsigned int flags);
>>>> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
>>>> unsigned long nr_segs, loff_t pos);
>>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>>> + struct file *filp, loff_t *ppos,
>>>> + size_t count, unsigned int flags);
>>>> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
>>>> unsigned long nr_segs, loff_t pos);
>>>> static int nfs_file_flush(struct file *, fl_owner_t id);
>>>> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
>>>> .lock = nfs_lock,
>>>> .flock = nfs_flock,
>>>> .splice_read = nfs_file_splice_read,
>>>> + .splice_write = nfs_file_splice_write,
>>>> .check_flags = nfs_check_flags,
>>>> .setlease = nfs_setlease,
>>>> };
>>>> @@ -550,6 +554,26 @@ out_swapfile:
>>>> goto out;
>>>> }
>>>>
>>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>>> + struct file *filp, loff_t *ppos,
>>>> + size_t count, unsigned int flags)
>>>> +{
>>>> + struct dentry *dentry = filp->f_path.dentry;
>>>> + struct inode *inode = dentry->d_inode;
>>>> +
>>>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>>>> + dentry->d_parent->d_name.name, dentry->d_name.name,
>>>> + (unsigned long) count, (unsigned long long) *ppos);
>>>> +
>>>> + if (IS_SWAPFILE(inode)) {
>>>> + printk(KERN_INFO "NFS: attempt to write to active swap"
>>>> + "file!\n");
>>>> + return -EBUSY;
>>>> + }
>>>> +
>>>> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
>>>> +}
>>>> +
>>>> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>>> {
>>>> struct inode *inode = filp->f_mapping->host;
>>>>
>>
>> --
>> Suresh Jayaraman
>>
>> _______________________________________________
>> ltt-dev mailing list
>> [email protected]
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


--
Suresh Jayaraman

2009-04-20 12:23:44

by Trond Myklebust

[permalink] [raw]
Subject: Re: [ltt-dev] [PATCH] nfs: add support for splice writes

On Mon, 2009-04-20 at 11:09 +0530, Suresh Jayaraman wrote:
> Hi Trond,
>
> Do you think this patch is OK? Can this be considered for merging?
>
> Thanks,
>
> Masahiro Tamori wrote:
> > Hello Suresh and Mathieu,
> >
> > 2009/4/2 Suresh Jayaraman <[email protected]>:
> >> Mathieu Desnoyers wrote:
> >>> * Suresh Jayaraman ([email protected]) wrote:
> >>>> This patch attempts to add splice writes support. In essence, it just
> >>>> calls generic_file_splice_write() after doing a little sanity check.
> >>>> This would allow LTTng users that are using NFS to store trace data.
> >>>> There could be more applications that could be benefitted too.
> >>>>
> >>>> I have tested this using the Jens' test program and have found no
> >>>> real issues. The test program is inlined below:
> >>>>
> >>> There is just a small checkpatch nit that I'll fix directly in place in
> >>> the LTTng tree.
> >>>
> >>> WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
> >>> #93: FILE: fs/nfs/file.c:564:
> >>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
> >>>
> >>> total: 0 errors, 1 warnings, 42 lines checked
> >>>
> >> Yes, I noticed it. There are quite a few places in nfs code where we
> >> happened to use that (that doesn't imply that it shouldn't be fixed), so
> >> I thought it's OK.
> >>
> >>> That's great ! I'll pull it in the LTTng tree so my users can try it.
> >>> They currently cannot write LTTng traces to NFS mounts anyway.
> >> Yes, please report the test results. Would appreciate it very much.
> >>
> >>
> >> Thanks,
> >
> > Thank you for creating a patch !!
> >
> > I tested it, and it works fine.
> >
> > My environment is following,
> > LTTV 0.12.10
> > LTTng 0.100
> > LTT Control 0.65
> > Kernles 2.6.29
> >
> > LTTng version is old now, but the test is passed on ARM11 target.
> >
> > Furthermore, I run the splice test and the test is passed too.
> > The test code is copied from
> > http://kzk9.net/blog/2007/05/splice2.html
> > (Japanese page)
> >
> > Thanks all people who commented this issue.
> >
> > Best regards,
> > Masahiro Tamori
> >
> >>>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> >>>> index 90f292b..13d6a00 100644
> >>>> --- a/fs/nfs/file.c
> >>>> +++ b/fs/nfs/file.c
> >>>> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
> >>>> size_t count, unsigned int flags);
> >>>> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
> >>>> unsigned long nr_segs, loff_t pos);
> >>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> >>>> + struct file *filp, loff_t *ppos,
> >>>> + size_t count, unsigned int flags);
> >>>> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
> >>>> unsigned long nr_segs, loff_t pos);
> >>>> static int nfs_file_flush(struct file *, fl_owner_t id);
> >>>> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
> >>>> .lock = nfs_lock,
> >>>> .flock = nfs_flock,
> >>>> .splice_read = nfs_file_splice_read,
> >>>> + .splice_write = nfs_file_splice_write,
> >>>> .check_flags = nfs_check_flags,
> >>>> .setlease = nfs_setlease,
> >>>> };
> >>>> @@ -550,6 +554,26 @@ out_swapfile:
> >>>> goto out;
> >>>> }
> >>>>
> >>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> >>>> + struct file *filp, loff_t *ppos,
> >>>> + size_t count, unsigned int flags)
> >>>> +{
> >>>> + struct dentry *dentry = filp->f_path.dentry;
> >>>> + struct inode *inode = dentry->d_inode;
> >>>> +
> >>>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
> >>>> + dentry->d_parent->d_name.name, dentry->d_name.name,
> >>>> + (unsigned long) count, (unsigned long long) *ppos);
> >>>> +
> >>>> + if (IS_SWAPFILE(inode)) {
> >>>> + printk(KERN_INFO "NFS: attempt to write to active swap"
> >>>> + "file!\n");
> >>>> + return -EBUSY;
> >>>> + }

I don't know that we really need this. We should sweep through the NFS
code and kill all those IS_SWAPFILE() thingys. Or at least #define
IS_SWAPFILE(a) (0)
...

> >>>> +
> >>>> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
> >>>> +}
> >>>> +
> >>>> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> >>>> {
> >>>> struct inode *inode = filp->f_mapping->host;
> >>>>

Otherwise it looks fine...

2009-04-02 06:29:58

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH] nfs: add support for splice writes

* Suresh Jayaraman ([email protected]) wrote:
> This patch attempts to add splice writes support. In essence, it just
> calls generic_file_splice_write() after doing a little sanity check.
> This would allow LTTng users that are using NFS to store trace data.
> There could be more applications that could be benefitted too.
>
> I have tested this using the Jens' test program and have found no
> real issues. The test program is inlined below:
>

That's great ! I'll pull it in the LTTng tree so my users can try it.
They currently cannot write LTTng traces to NFS mounts anyway.

Thanks !

Mathieu

> /*
> * splice-out.c: Splice stdout to file
> */
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <fcntl.h>
>
> #define SPLICE_SIZE (64*1024)
>
> int main(int argc, char *argv[])
> {
> int fd;
>
> if (argc < 2) {
> printf("%s: outfile\n", argv[0]);
> return 1;
> }
>
> fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC, 0644);
> if (fd < 0) {
> perror("open");
> return 1;
> }
>
> do {
> int ret = splice(STDIN_FILENO, NULL, fd, NULL, SPLICE_SIZE, 0);
>
> if (ret < 0) {
> perror("splice");
> break;
> } else if (ret < SPLICE_SIZE)
> break;
> } while (1);
>
> close(fd);
> return 0;
> }
>
> Compile with -D _GNU_SOURCE and do something like:
> echo "some stuff" | ./splice-out <outfile>
>
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
> fs/nfs/file.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 90f292b..13d6a00 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
> size_t count, unsigned int flags);
> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
> unsigned long nr_segs, loff_t pos);
> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> + struct file *filp, loff_t *ppos,
> + size_t count, unsigned int flags);
> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
> unsigned long nr_segs, loff_t pos);
> static int nfs_file_flush(struct file *, fl_owner_t id);
> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
> .lock = nfs_lock,
> .flock = nfs_flock,
> .splice_read = nfs_file_splice_read,
> + .splice_write = nfs_file_splice_write,
> .check_flags = nfs_check_flags,
> .setlease = nfs_setlease,
> };
> @@ -550,6 +554,26 @@ out_swapfile:
> goto out;
> }
>
> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> + struct file *filp, loff_t *ppos,
> + size_t count, unsigned int flags)
> +{
> + struct dentry *dentry = filp->f_path.dentry;
> + struct inode *inode = dentry->d_inode;
> +
> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
> + dentry->d_parent->d_name.name, dentry->d_name.name,
> + (unsigned long) count, (unsigned long long) *ppos);
> +
> + if (IS_SWAPFILE(inode)) {
> + printk(KERN_INFO "NFS: attempt to write to active swap"
> + "file!\n");
> + return -EBUSY;
> + }
> +
> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
> +}
> +
> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> {
> struct inode *inode = filp->f_mapping->host;
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2009-04-02 06:32:39

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH] nfs: add support for splice writes

* Suresh Jayaraman ([email protected]) wrote:
> This patch attempts to add splice writes support. In essence, it just
> calls generic_file_splice_write() after doing a little sanity check.
> This would allow LTTng users that are using NFS to store trace data.
> There could be more applications that could be benefitted too.
>
> I have tested this using the Jens' test program and have found no
> real issues. The test program is inlined below:
>

There is just a small checkpatch nit that I'll fix directly in place in
the LTTng tree.

WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
#93: FILE: fs/nfs/file.c:564:
+ dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",

total: 0 errors, 1 warnings, 42 lines checked

Mathieu

> /*
> * splice-out.c: Splice stdout to file
> */
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <fcntl.h>
>
> #define SPLICE_SIZE (64*1024)
>
> int main(int argc, char *argv[])
> {
> int fd;
>
> if (argc < 2) {
> printf("%s: outfile\n", argv[0]);
> return 1;
> }
>
> fd = open(argv[1], O_WRONLY | O_CREAT | O_TRUNC, 0644);
> if (fd < 0) {
> perror("open");
> return 1;
> }
>
> do {
> int ret = splice(STDIN_FILENO, NULL, fd, NULL, SPLICE_SIZE, 0);
>
> if (ret < 0) {
> perror("splice");
> break;
> } else if (ret < SPLICE_SIZE)
> break;
> } while (1);
>
> close(fd);
> return 0;
> }
>
> Compile with -D _GNU_SOURCE and do something like:
> echo "some stuff" | ./splice-out <outfile>
>
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
> fs/nfs/file.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 90f292b..13d6a00 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
> size_t count, unsigned int flags);
> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
> unsigned long nr_segs, loff_t pos);
> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> + struct file *filp, loff_t *ppos,
> + size_t count, unsigned int flags);
> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
> unsigned long nr_segs, loff_t pos);
> static int nfs_file_flush(struct file *, fl_owner_t id);
> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
> .lock = nfs_lock,
> .flock = nfs_flock,
> .splice_read = nfs_file_splice_read,
> + .splice_write = nfs_file_splice_write,
> .check_flags = nfs_check_flags,
> .setlease = nfs_setlease,
> };
> @@ -550,6 +554,26 @@ out_swapfile:
> goto out;
> }
>
> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
> + struct file *filp, loff_t *ppos,
> + size_t count, unsigned int flags)
> +{
> + struct dentry *dentry = filp->f_path.dentry;
> + struct inode *inode = dentry->d_inode;
> +
> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
> + dentry->d_parent->d_name.name, dentry->d_name.name,
> + (unsigned long) count, (unsigned long long) *ppos);
> +
> + if (IS_SWAPFILE(inode)) {
> + printk(KERN_INFO "NFS: attempt to write to active swap"
> + "file!\n");
> + return -EBUSY;
> + }
> +
> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
> +}
> +
> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
> {
> struct inode *inode = filp->f_mapping->host;
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2009-04-02 06:42:20

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [PATCH] nfs: add support for splice writes

Mathieu Desnoyers wrote:
> * Suresh Jayaraman ([email protected]) wrote:
>> This patch attempts to add splice writes support. In essence, it just
>> calls generic_file_splice_write() after doing a little sanity check.
>> This would allow LTTng users that are using NFS to store trace data.
>> There could be more applications that could be benefitted too.
>>
>> I have tested this using the Jens' test program and have found no
>> real issues. The test program is inlined below:
>>
>
> There is just a small checkpatch nit that I'll fix directly in place in
> the LTTng tree.
>
> WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
> #93: FILE: fs/nfs/file.c:564:
> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>
> total: 0 errors, 1 warnings, 42 lines checked
>

Yes, I noticed it. There are quite a few places in nfs code where we
happened to use that (that doesn't imply that it shouldn't be fixed), so
I thought it's OK.

> That's great ! I'll pull it in the LTTng tree so my users can try it.
> They currently cannot write LTTng traces to NFS mounts anyway.

Yes, please report the test results. Would appreciate it very much.


Thanks,

>>
>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>> index 90f292b..13d6a00 100644
>> --- a/fs/nfs/file.c
>> +++ b/fs/nfs/file.c
>> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
>> size_t count, unsigned int flags);
>> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
>> unsigned long nr_segs, loff_t pos);
>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>> + struct file *filp, loff_t *ppos,
>> + size_t count, unsigned int flags);
>> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
>> unsigned long nr_segs, loff_t pos);
>> static int nfs_file_flush(struct file *, fl_owner_t id);
>> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
>> .lock = nfs_lock,
>> .flock = nfs_flock,
>> .splice_read = nfs_file_splice_read,
>> + .splice_write = nfs_file_splice_write,
>> .check_flags = nfs_check_flags,
>> .setlease = nfs_setlease,
>> };
>> @@ -550,6 +554,26 @@ out_swapfile:
>> goto out;
>> }
>>
>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>> + struct file *filp, loff_t *ppos,
>> + size_t count, unsigned int flags)
>> +{
>> + struct dentry *dentry = filp->f_path.dentry;
>> + struct inode *inode = dentry->d_inode;
>> +
>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>> + dentry->d_parent->d_name.name, dentry->d_name.name,
>> + (unsigned long) count, (unsigned long long) *ppos);
>> +
>> + if (IS_SWAPFILE(inode)) {
>> + printk(KERN_INFO "NFS: attempt to write to active swap"
>> + "file!\n");
>> + return -EBUSY;
>> + }
>> +
>> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
>> +}
>> +
>> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>> {
>> struct inode *inode = filp->f_mapping->host;
>>
>


--
Suresh Jayaraman

2009-04-02 12:42:05

by Masahiro Tamori

[permalink] [raw]
Subject: Re: [ltt-dev] [PATCH] nfs: add support for splice writes

Hello Suresh and Mathieu,

2009/4/2 Suresh Jayaraman <[email protected]>:
> Mathieu Desnoyers wrote:
>> * Suresh Jayaraman ([email protected]) wrote:
>>> This patch attempts to add splice writes support. In essence, it just
>>> calls generic_file_splice_write() after doing a little sanity check.
>>> This would allow LTTng users that are using NFS to store trace data.
>>> There could be more applications that could be benefitted too.
>>>
>>> I have tested this using the Jens' test program and have found no
>>> real issues. The test program is inlined below:
>>>
>>
>> There is just a small checkpatch nit that I'll fix directly in place in
>> the LTTng tree.
>>
>> WARNING: %Ld/%Lu are not-standard C, use %lld/%llu
>> #93: FILE: fs/nfs/file.c:564:
>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>>
>> total: 0 errors, 1 warnings, 42 lines checked
>>
>
> Yes, I noticed it. There are quite a few places in nfs code where we
> happened to use that (that doesn't imply that it shouldn't be fixed), so
> I thought it's OK.
>
>> That's great ! I'll pull it in the LTTng tree so my users can try it.
>> They currently cannot write LTTng traces to NFS mounts anyway.
>
> Yes, please report the test results. Would appreciate it very much.
>
>
> Thanks,

Thank you for creating a patch !!

I tested it, and it works fine.

My environment is following,
LTTV 0.12.10
LTTng 0.100
LTT Control 0.65
Kernles 2.6.29

LTTng version is old now, but the test is passed on ARM11 target.

Furthermore, I run the splice test and the test is passed too.
The test code is copied from
http://kzk9.net/blog/2007/05/splice2.html
(Japanese page)

Thanks all people who commented this issue.

Best regards,
Masahiro Tamori

>
>>>
>>> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
>>> index 90f292b..13d6a00 100644
>>> --- a/fs/nfs/file.c
>>> +++ b/fs/nfs/file.c
>>> @@ -47,6 +47,9 @@ static ssize_t nfs_file_splice_read(struct file *filp, loff_t *ppos,
>>> size_t count, unsigned int flags);
>>> static ssize_t nfs_file_read(struct kiocb *, const struct iovec *iov,
>>> unsigned long nr_segs, loff_t pos);
>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>> + struct file *filp, loff_t *ppos,
>>> + size_t count, unsigned int flags);
>>> static ssize_t nfs_file_write(struct kiocb *, const struct iovec *iov,
>>> unsigned long nr_segs, loff_t pos);
>>> static int nfs_file_flush(struct file *, fl_owner_t id);
>>> @@ -76,6 +79,7 @@ const struct file_operations nfs_file_operations = {
>>> .lock = nfs_lock,
>>> .flock = nfs_flock,
>>> .splice_read = nfs_file_splice_read,
>>> + .splice_write = nfs_file_splice_write,
>>> .check_flags = nfs_check_flags,
>>> .setlease = nfs_setlease,
>>> };
>>> @@ -550,6 +554,26 @@ out_swapfile:
>>> goto out;
>>> }
>>>
>>> +static ssize_t nfs_file_splice_write(struct pipe_inode_info *pipe,
>>> + struct file *filp, loff_t *ppos,
>>> + size_t count, unsigned int flags)
>>> +{
>>> + struct dentry *dentry = filp->f_path.dentry;
>>> + struct inode *inode = dentry->d_inode;
>>> +
>>> + dprintk("NFS splice_write(%s/%s, %lu@%Lu)\n",
>>> + dentry->d_parent->d_name.name, dentry->d_name.name,
>>> + (unsigned long) count, (unsigned long long) *ppos);
>>> +
>>> + if (IS_SWAPFILE(inode)) {
>>> + printk(KERN_INFO "NFS: attempt to write to active swap"
>>> + "file!\n");
>>> + return -EBUSY;
>>> + }
>>> +
>>> + return generic_file_splice_write(pipe, filp, ppos, count, flags);
>>> +}
>>> +
>>> static int do_getlk(struct file *filp, int cmd, struct file_lock *fl)
>>> {
>>> struct inode *inode = filp->f_mapping->host;
>>>
>>
>
>
> --
> Suresh Jayaraman
>
> _______________________________________________
> ltt-dev mailing list
> [email protected]
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>