| From | Sent On | Attachments |
|---|---|---|
| Giovanni Trematerra | Jan 7, 2012 6:35 pm | |
| John Baldwin | Jan 9, 2012 5:48 am | |
| Bruce Evans | Jan 9, 2012 6:34 am | |
| Kostik Belousov | Jan 9, 2012 6:46 am | |
| Giovanni Trematerra | Jan 9, 2012 2:27 pm | |
| Giovanni Trematerra | Jan 9, 2012 3:19 pm | |
| Bruce Evans | Jan 10, 2012 4:03 am | |
| Giovanni Trematerra | Jan 10, 2012 3:04 pm | |
| Giovanni Trematerra | Jan 10, 2012 3:33 pm | |
| Giovanni Trematerra | Jan 10, 2012 4:14 pm | |
| Giovanni Trematerra | Jan 12, 2012 12:55 am | |
| Bruce Evans | Jan 12, 2012 4:51 am | |
| Giovanni Trematerra | Jan 17, 2012 3:42 am | |
| Jilles Tjoelker | Jan 28, 2012 3:36 pm | |
| Bruce Evans | Jan 29, 2012 3:31 am | |
| Giovanni Trematerra | Jan 31, 2012 1:48 pm |
| Subject: | Re: pipe/fifo code merged. | |
|---|---|---|
| From: | John Baldwin (jh...@freebsd.org) | |
| Date: | Jan 9, 2012 5:48:02 am | |
| List: | org.freebsd.freebsd-arch | |
On Saturday, January 07, 2012 9:35:47 pm Giovanni Trematerra wrote:
Hi, the patch at http://www.trematerra.net/patches/pipefifo_merge2.diff
is a preliminary version of the FIFO optimizations project that I picked up from the wiki. http://wiki.freebsd.org/IdeasPage#FIFO_optimizations_.28GSoC.29
zhaoshuai@ produced the following patch in the 2009 which attempted a first merge of the interfaces: http://www.trematerra.net/patches/fifo_soc2009.diff
However I felt like the work was not yet completed and come up with my final version. Now fifoes derive their structures from pipes one with just special handling to support VFS operations. All the operations but the creation/destruction for fifoes and pipes are handled by the same code. The heart of the patch is the new struct pipeinfo. pipeinfo is a per-file descriptor state. Basically it maintains a read end and a write end for the descriptor. As pipes are bidirectional in FreeBSD, for a pipe this two fields are always equal but different for a fifo. To let fifo code in sys/fs/fifofs/fifo_vnops.c create/destroy the pipe, two functions (pipe_ctor/pipe_dtor) were written. pipe_ctor setups things like a call to kern_pipe and return a pipeinfo structure, while pipe_dtor releases all the resources for a given pipeinfo. Once a pipe was setup during a fifo_open call, all the subsequent operations on the fifo are handled by the same code of a pipe expect for the clean up code that calls pipe_dtor. Allocation of two pipeinfo structures for a pipe were showed to slow down things by some micro-benchmarking. To speed up things during creation/destruction of pipes, the patch allocates all the needed data structure zone using the umapipe struct that packing together all the needed data structures to be allocated at pipe creation. A similar umafifo structure is used for fifoes. Thanks to jilles that made a review of the patch in a previous form, privately. Thanks a lot to attilio that answered my stupid questions and drove me in the right direction.
Thanks for taking this on. In general I think this looks good, but had a few comments:
- Why did you move setting the timestamps of pipes from the UMA ctor to an init routine? This seems wrong. The init routine is only invoked when memory is first allocated to a slab to create a set of umapipe or umafifo structures. However, that umapipe/fifo may be reused multiple times, all with the same timestamp. Setting the timestamp in the ctor routine means it is set each time a pipepair or fifo is created which seems more appropiate. Similarly with the inode value. - I would maybe call pipe_ctor(), fifo_ctor() instead or otherwise adjust the name to note it is only used to create a FIFO, not used to create a normal pipe pair (which it's name implies). - s/socket/pipe/ in the FIONREAD comment in sys_pipe.c. - Two extra blank lines in the patch that I think should be reverted:
--- a/sys/fs/fifofs/fifo_vnops.c Sun Jan 01 19:00:13 2012 +0100 +++ b/sys/fs/fifofs/fifo_vnops.c Mon Jan 09 00:13:55 2012 +0100 int fi_wgen; };
+ static vop_print_t fifo_print; static vop_open_t fifo_open; .... @@ -1598,13 +1839,20 @@ pipe_kqfilter(struct file *fp, struct kn return (EPIPE); } cpipe = cpipe->pipe_peer; + break; default: PIPE_UNLOCK(cpipe); return (EINVAL);
-- John Baldwin
_______________________________________________ free...@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-arch To unsubscribe, send any mail to "free...@freebsd.org"





