Re: [PATCH] splice support #2

[Jens Axboe]

On Thu, Mar 30 2006, Jens Axboe wrote:
> On Thu, Mar 30 2006, Jens Axboe wrote:
> > On Thu, Mar 30 2006, Andrew Morton wrote:
> > > Jens Axboe <axboe@xxxxxxx> wrote:
> > > >
> > > > > > +static void *page_cache_pipe_buf_map(struct file *file,
> > > >  > > +				     struct pipe_inode_info *info,
> > > >  > > +				     struct pipe_buffer *buf)
> > > >  > > +{
> > > >  > > +	struct page *page = buf->page;
> > > >  > > +
> > > >  > > +	lock_page(page);
> > > >  > > +
> > > >  > > +	if (!PageUptodate(page)) {
> > > >  > 
> > > >  >  || page->mapping == NULL
> > > > 
> > > >  Fixed
> > > 
> > > Actually that wasn't right.  If the page was truncated then we shouldn't
> > > return -EIO to userspace.  We should return zero, as this is the first
> > > page.
> > 
> > _if_ this is the first page, then agree.
> > 
> > > Which means either returning an ERR_PTR here for -EIO or re-checking i_size
> > > in the caller.   Maybe the latter?
> > 
> > The latter sounds better, I'll add that.
> 
> Actually that's a little hard, since you don't know what state buf->page
> is in - it might be page cache, it might not be. So I opted for the
> ERR_PTR approach, just returning -ENODATA on !page->mapping.

Ala

diff --git a/fs/splice.c b/fs/splice.c
index 7927dff..68b98bb 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -47,9 +47,14 @@ static void *page_cache_pipe_buf_map(str
 
 	lock_page(page);
 
-	if (!PageUptodate(page) || !page->mapping) {
+	if (!PageUptodate(page)) {
+		unlock_page(page);
+		return ERR_PTR(-EIO);
+	}
+
+	if (!page->mapping) {
 		unlock_page(page);
-		return NULL;
+		return ERR_PTR(-ENODATA);
 	}
 
 	return kmap(buf->page);
@@ -279,6 +293,7 @@ static int pipe_to_sendpage(struct pipe_
 	loff_t pos = sd->pos;
 	unsigned int offset;
 	ssize_t ret;
+	void *ptr;
 
 	/*
 	 * sub-optimal, but we are limited by the pipe ->map. we don't
@@ -286,8 +301,9 @@ static int pipe_to_sendpage(struct pipe_
 	 * have the page pinned if the pipe page originates from the
 	 * page cache
 	 */
-	if (!buf->ops->map(file, info, buf))
-		return -EIO;
+	ptr = buf->ops->map(file, info, buf);
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
 
 	offset = pos & ~PAGE_CACHE_MASK;
 
@@ -334,8 +350,8 @@ static int pipe_to_file(struct pipe_inod
 	 * after this, page will be locked and unmapped
 	 */
 	src = buf->ops->map(file, info, buf);
-	if (!src)
-		return -EIO;
+	if (IS_ERR(src))
+		return PTR_ERR(src);
 
 	index = sd->pos >> PAGE_CACHE_SHIFT;
 	offset = sd->pos & ~PAGE_CACHE_MASK;
@@ -436,7 +452,7 @@ static ssize_t move_from_pipe(struct ino
 
 			err = actor(info, buf, &sd);
 			if (err) {
-				if (!ret)
+				if (!ret && err != -ENODATA)
 					ret = err;
 
 				break;

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/