Re: [PATCH][RFC] dynamic pipe resizing
On Saturday 15 December 2007, Jan Engelhardt wrote: > On Aug 24 2007 10:52, Jens Axboe wrote: > >Subject: [PATCH][RFC] dynamic pipe resizing > >Like with my original splice patches from 2005, I used fcntl() > >F_GETPIPE_SZ and F_SETPIPE_SZ to change the size of the pipe. I'm not > >particularly fond of that interface, so suggestions on how to improve it > >would be appreciated. Even if fcntl() should be the preferred approach, > >I think it would be better to pass in a byte based value instead of a > >number of pages. > > > Could this patch still make it in? > Yes, I think its set() and get() parts should use bytes and convert > to/from pages. > > Perhaps just round up, and mention the rounding in the manpage, so that > noone gets a shock when the pipe is not exactly as small as requested. Yes, but document only that it is rounding and make the unit arbitrary. That reduces the ABI requirements. Best Regards Ingo Oeser -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] dynamic pipe resizing
On Aug 24 2007 10:52, Jens Axboe wrote:
>Subject: [PATCH][RFC] dynamic pipe resizing
>
>Hi,
>
>Dabbling around with splice a bit, I added some code to change the size
>of a pipe. Currently it's hardcoded as 16 pages, with this patch you can
>shrink (if you wanted) or grow (the likely scenario) if you want to
>increase the size of your in-kernel buffer for splice operations.
>
>Like with my original splice patches from 2005, I used fcntl()
>F_GETPIPE_SZ and F_SETPIPE_SZ to change the size of the pipe. I'm not
>particularly fond of that interface, so suggestions on how to improve it
>would be appreciated. Even if fcntl() should be the preferred approach,
>I think it would be better to pass in a byte based value instead of a
>number of pages.
>
Could this patch still make it in?
Yes, I think its set() and get() parts should use bytes and convert
to/from pages.
> /*
>+ * Allocate a new array of pipe buffers and copy the info over. Returns the
>+ * pipe size if successful, or return -ERROR on error.
>+ */
>+static long pipe_set_size(struct pipe_inode_info *pipe, unsigned long arg)
>+{
>+ struct pipe_buffer *bufs;
>+
>+ /*
>+ * Must be a power-of-2 current
>+ */
>+ if (arg & (arg - 1))
>+ return -EINVAL;
>+
Perhaps just round up, and mention the rounding in the manpage, so that
noone gets a shock when the pipe is not exactly as small as requested.
>+ /*
>+ * We can shrink the pipe, if arg >= pipe->nrbufs. Since we don't
>+ * expect a lot of shrink+grow operations, just free and allocate
>+ * again like we would do for growing. If the pipe currently
>+ * contains more buffers than arg, then return busy.
>+ */
>+ if (arg < pipe->nrbufs)
>+ return -EBUSY;
How big can a pipe become? k*alloc has a certain limit, so should
perhaps vmalloc be used for when kalloc fails?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] dynamic pipe resizing
On Fri, Aug 24 2007, Jan Engelhardt wrote:
>
> On Aug 24 2007 10:52, Jens Axboe wrote:
> >Hi,
> >
> >Dabbling around with splice a bit, I added some code to change the size
> >of a pipe. Currently it's hardcoded as 16 pages, with this patch you can
> >shrink (if you wanted) or grow (the likely scenario) if you want to
> >increase the size of your in-kernel buffer for splice operations.
>
> Wonderful, I just did the same {-.-}
> (but then dropped the patch because it did not help my application at all,
> so never mind).
>
> >Like with my original splice patches from 2005, I used fcntl()
> >F_GETPIPE_SZ and F_SETPIPE_SZ to change the size of the pipe. I'm not
> >particularly fond of that interface, so suggestions on how to improve it
> >would be appreciated. Even if fcntl() should be the preferred approach,
> >I think it would be better to pass in a byte based value instead of a
> >number of pages.
>
> And using % pipe->buffers then?
I'd probably prefer sticking to power-of-2 array sizes, so just round it
up appropriately. But it's likely not a big deal.
--
Jens Axboe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] dynamic pipe resizing
On Aug 24 2007 10:52, Jens Axboe wrote:
>Hi,
>
>Dabbling around with splice a bit, I added some code to change the size
>of a pipe. Currently it's hardcoded as 16 pages, with this patch you can
>shrink (if you wanted) or grow (the likely scenario) if you want to
>increase the size of your in-kernel buffer for splice operations.
Wonderful, I just did the same {-.-}
(but then dropped the patch because it did not help my application at all,
so never mind).
>Like with my original splice patches from 2005, I used fcntl()
>F_GETPIPE_SZ and F_SETPIPE_SZ to change the size of the pipe. I'm not
>particularly fond of that interface, so suggestions on how to improve it
>would be appreciated. Even if fcntl() should be the preferred approach,
>I think it would be better to pass in a byte based value instead of a
>number of pages.
And using % pipe->buffers then?
Jan
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH][RFC] dynamic pipe resizing
Hi,
Dabbling around with splice a bit, I added some code to change the size
of a pipe. Currently it's hardcoded as 16 pages, with this patch you can
shrink (if you wanted) or grow (the likely scenario) if you want to
increase the size of your in-kernel buffer for splice operations.
Like with my original splice patches from 2005, I used fcntl()
F_GETPIPE_SZ and F_SETPIPE_SZ to change the size of the pipe. I'm not
particularly fond of that interface, so suggestions on how to improve it
would be appreciated. Even if fcntl() should be the preferred approach,
I think it would be better to pass in a byte based value instead of a
number of pages.
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 78b2ff0..c4571fc 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -376,6 +377,10 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned
long arg,
case F_NOTIFY:
err = fcntl_dirnotify(fd, filp, arg);
break;
+ case F_SETPIPE_SZ:
+ case F_GETPIPE_SZ:
+ err = pipe_fcntl(filp, cmd, arg);
+ break;
default:
break;
}
diff --git a/fs/pipe.c b/fs/pipe.c
index 6b3d91a..734ede4 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -342,7 +342,7 @@ redo:
if (!buf->len) {
buf->ops = NULL;
ops->release(pipe, buf);
- curbuf = (curbuf + 1) & (PIPE_BUFFERS-1);
+ curbuf = (curbuf + 1) & (pipe->buffers - 1);
pipe->curbuf = curbuf;
pipe->nrbufs = --bufs;
do_wakeup = 1;
@@ -424,7 +424,7 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov,
chars = total_len & (PAGE_SIZE-1); /* size of the last buffer */
if (pipe->nrbufs && chars != 0) {
int lastbuf = (pipe->curbuf + pipe->nrbufs - 1) &
- (PIPE_BUFFERS-1);
+ (pipe->buffers - 1);
struct pipe_buffer *buf = pipe->bufs + lastbuf;
const struct pipe_buf_operations *ops = buf->ops;
int offset = buf->offset + buf->len;
@@ -470,8 +470,8 @@ redo1:
break;
}
bufs = pipe->nrbufs;
- if (bufs < PIPE_BUFFERS) {
- int newbuf = (pipe->curbuf + bufs) & (PIPE_BUFFERS-1);
+ if (bufs < pipe->buffers) {
+ int newbuf = (pipe->curbuf + bufs) & (pipe->buffers-1);
struct pipe_buffer *buf = pipe->bufs + newbuf;
struct page *page = pipe->tmp_page;
char *src;
@@ -532,7 +532,7 @@ redo2:
if (!total_len)
break;
}
- if (bufs < PIPE_BUFFERS)
+ if (bufs < pipe->buffers)
continue;
if (filp->f_flags & O_NONBLOCK) {
if (!ret)
@@ -594,7 +594,7 @@ pipe_ioctl(struct inode *pino, struct file *filp,
nrbufs = pipe->nrbufs;
while (--nrbufs >= 0) {
count += pipe->bufs[buf].len;
- buf = (buf+1) & (PIPE_BUFFERS-1);
+ buf = (buf+1) & (pipe->buffers - 1);
}
mutex_unlock(&inode->i_mutex);
@@ -625,7 +625,7 @@ pipe_poll(struct file *filp, poll_table *wait)
}
if (filp->f_mode & FMODE_WRITE) {
- mask |= (nrbufs < PIPE_BUFFERS) ? POLLOUT | POLLWRNORM : 0;
+ mask |= (nrbufs < pipe->buffers) ? POLLOUT | POLLWRNORM : 0;
/*
* Most Unices do not set POLLERR for FIFOs but on Linux they
* behave exactly like pipes for poll().
@@ -860,25 +860,32 @@ struct pipe_inode_info * alloc_pipe_info(struct inode
*inode)
pipe = kzalloc(sizeof(struct pipe_inode_info), GFP_KERNEL);
if (pipe) {
- init_waitqueue_head(&pipe->wait);
- pipe->r_counter = pipe->w_counter = 1;
- pipe->inode = inode;
+ pipe->bufs = kzalloc(sizeof(struct pipe_buffer) *
PIPE_DEF_BUFFERS, GFP_KERNEL);
+ if (pipe->bufs) {
+ init_waitqueue_head(&pipe->wait);
+ pipe->r_counter = pipe->w_counter = 1;
+ pipe->inode = inode;
+ pipe->buffers = PIPE_DEF_BUFFERS;
+ return pipe;
+ }
+ kfree(pipe);
}
- return pipe;
+ return NULL;
}
void __free_pipe_info(struct pipe_inode_info *pipe)
{
int i;
- for (i
