[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070601080125.GM32105@kernel.dk>
Date: Fri, 1 Jun 2007 10:01:25 +0200
From: Jens Axboe <jens.axboe@...cle.com>
To: Neil Brown <neilb@...e.de>
Cc: Christoph Hellwig <hch@...radead.org>,
linux-kernel@...r.kernel.org, cotte@...ibm.com, hugh@...itas.com,
zanussi@...ibm.com, Linus Torvalds <torvalds@...l.org>
Subject: Re: [PATCH] sendfile removal (nfsd update)
On Fri, Jun 01 2007, Jens Axboe wrote:
> On Fri, Jun 01 2007, Neil Brown wrote:
> >
> > Ok, here is a patch that makes nfsd use splice instead of sendfile.
> > It appears to both compile and work.
> >
> > Some observations:
> > - __splice_from_pipe wants a "struct file*" and I wanted to pass a
> > "struct svcrqst *". Maybe it should take a void * ?
> > - It also wants a *ppos which I had no use for.. It that really
> > need? Cannot &file->f_pos be used?
>
> See:
>
> http://git.kernel.dk/?p=linux-2.6-block.git;a=commitdiff;h=c73a9509ef7877d31e0c97c684ee8b7ed13ecbbe;hp=07f0e716250d4a3a550b2f39bd0a7e4e6566b3c2
>
> I'll rebase the conversion on top of this one, loop will need the same
> change.
>
> > - I copied do_splice_to from splice.c as it wasn't exported, and
> > then found I couldn't compile because rw_verify_area wasn't
> > exported. As nfsd doesn't need that (we never export
> > mandatory-locking files) I just remove it and some other cruft
> > that I didn't need.... Not sure if that was the best approach.
> > - I needed to export alloc_pipe_info. Maybe there should be a
> > get_current_pipe instead which does the alloc if needed.
> > - I would much rather have something like free_pipe_info exported
> > than open code it in do_splice_read (which is based heavily on
> > do_splice_direct).
>
> I need the same thing for loop. I think I'll add do_splice_foo() to
> fs/splice.c in a way that loop can use instead of do_splice_direct(),
> then nfsd should be able to do the same.
I rebased and added a splice_direct_to_actor() helper that nfsd can then
use. This makes the nfsd diff look like the below, which I think you'll
agree is a lot more pleasant. You can view the tree here:
http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=splice
The vmsplice patch probably needs to be split in two, for now it is the
one introducing the framework that nfsd (and next, loop) can use.
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7e6aa24..96f76e2 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -23,7 +23,7 @@
#include <linux/file.h>
#include <linux/mount.h>
#include <linux/major.h>
-#include <linux/ext2_fs.h>
+#include <linux/pipe_fs_i.h>
#include <linux/proc_fs.h>
#include <linux/stat.h>
#include <linux/fcntl.h>
@@ -801,26 +801,32 @@ found:
}
/*
- * Grab and keep cached pages assosiated with a file in the svc_rqst
- * so that they can be passed to the netowork sendmsg/sendpage routines
- * directrly. They will be released after the sending has completed.
+ * Grab and keep cached pages associated with a file in the svc_rqst
+ * so that they can be passed to the network sendmsg/sendpage routines
+ * directly. They will be released after the sending has completed.
*/
static int
-nfsd_read_actor(read_descriptor_t *desc, struct page *page, unsigned long offset , unsigned long size)
+nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
+ struct splice_desc *sd)
{
- unsigned long count = desc->count;
- struct svc_rqst *rqstp = desc->arg.data;
+ struct svc_rqst *rqstp = sd->data;
struct page **pp = rqstp->rq_respages + rqstp->rq_resused;
+ struct page *page = buf->page;
+ size_t size;
+ int ret;
+
+ ret = buf->ops->pin(pipe, buf);
+ if (unlikely(ret))
+ return ret;
- if (size > count)
- size = count;
+ size = sd->len;
if (rqstp->rq_res.page_len == 0) {
get_page(page);
put_page(*pp);
*pp = page;
rqstp->rq_resused++;
- rqstp->rq_res.page_base = offset;
+ rqstp->rq_res.page_base = buf->offset;
rqstp->rq_res.page_len = size;
} else if (page != pp[-1]) {
get_page(page);
@@ -832,11 +838,15 @@ nfsd_read_actor(read_descriptor_t *desc, struct page *page, unsigned long offset
} else
rqstp->rq_res.page_len += size;
- desc->count = count - size;
- desc->written += size;
return size;
}
+static int nfsd_direct_splice_actor(struct pipe_inode_info *pipe,
+ struct splice_desc *sd)
+{
+ return __splice_from_pipe(pipe, sd, nfsd_splice_actor);
+}
+
static __be32
nfsd_vfs_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
loff_t offset, struct kvec *vec, int vlen, unsigned long *count)
@@ -861,10 +871,15 @@ nfsd_vfs_read(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
if (ra && ra->p_set)
file->f_ra = ra->p_ra;
- if (file->f_op->sendfile && rqstp->rq_sendfile_ok) {
- rqstp->rq_resused = 1;
- host_err = file->f_op->sendfile(file, &offset, *count,
- nfsd_read_actor, rqstp);
+ if (file->f_op->splice_read && rqstp->rq_splice_ok) {
+ struct splice_desc sd = {
+ .len = 0,
+ .total_len = *count,
+ .pos = offset,
+ };
+
+ sd.data = rqstp;
+ host_err = splice_direct_to_actor(file, &sd, nfsd_direct_splice_actor);
} else {
oldfs = get_fs();
set_fs(KERNEL_DS);
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists