Author: kib Date: Mon Mar 9 21:55:26 2020 New Revision: 358825 URL: https://svnweb.freebsd.org/changeset/base/358825
Log: Preallocate pipe buffers on pipe creation. Return ENOMEM if one of the buffer cannot be created even with the minimal size. This should avoid subsequent spurious ENOMEM errors from write(2) when buffer cannot be allocated on the fly, after we reported that the pipe was create succesfully. Reported by: Keno Fischer <k...@juliacomputing.com> Reviewed by: markj (previous version) Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D23993 Modified: head/sys/fs/fifofs/fifo_vnops.c head/sys/kern/sys_pipe.c head/sys/sys/pipe.h Modified: head/sys/fs/fifofs/fifo_vnops.c ============================================================================== --- head/sys/fs/fifofs/fifo_vnops.c Mon Mar 9 21:01:22 2020 (r358824) +++ head/sys/fs/fifofs/fifo_vnops.c Mon Mar 9 21:55:26 2020 (r358825) @@ -151,7 +151,9 @@ fifo_open(ap) if (fp == NULL || (ap->a_mode & FEXEC) != 0) return (EINVAL); if ((fip = vp->v_fifoinfo) == NULL) { - pipe_named_ctor(&fpipe, td); + error = pipe_named_ctor(&fpipe, td); + if (error != 0) + return (error); fip = malloc(sizeof(*fip), M_VNODE, M_WAITOK); fip->fi_pipe = fpipe; fpipe->pipe_wgen = fip->fi_readers = fip->fi_writers = 0; Modified: head/sys/kern/sys_pipe.c ============================================================================== --- head/sys/kern/sys_pipe.c Mon Mar 9 21:01:22 2020 (r358824) +++ head/sys/kern/sys_pipe.c Mon Mar 9 21:55:26 2020 (r358825) @@ -226,8 +226,8 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, piperesizeallowed, CTL static void pipeinit(void *dummy __unused); static void pipeclose(struct pipe *cpipe); static void pipe_free_kmem(struct pipe *cpipe); -static void pipe_create(struct pipe *pipe, int backing); -static void pipe_paircreate(struct thread *td, struct pipepair **p_pp); +static int pipe_create(struct pipe *pipe, bool backing); +static int pipe_paircreate(struct thread *td, struct pipepair **p_pp); static __inline int pipelock(struct pipe *cpipe, int catch); static __inline void pipeunlock(struct pipe *cpipe); #ifndef PIPE_NODIRECT @@ -335,11 +335,12 @@ pipe_zone_fini(void *mem, int size) mtx_destroy(&pp->pp_mtx); } -static void +static int pipe_paircreate(struct thread *td, struct pipepair **p_pp) { struct pipepair *pp; struct pipe *rpipe, *wpipe; + int error; *p_pp = pp = uma_zalloc(pipe_zone, M_WAITOK); #ifdef MAC @@ -357,22 +358,44 @@ pipe_paircreate(struct thread *td, struct pipepair **p knlist_init_mtx(&rpipe->pipe_sel.si_note, PIPE_MTX(rpipe)); knlist_init_mtx(&wpipe->pipe_sel.si_note, PIPE_MTX(wpipe)); - /* Only the forward direction pipe is backed by default */ - pipe_create(rpipe, 1); - pipe_create(wpipe, 0); + /* + * Only the forward direction pipe is backed by big buffer by + * default. + */ + error = pipe_create(rpipe, true); + if (error != 0) + goto fail; + error = pipe_create(wpipe, false); + if (error != 0) { + pipe_free_kmem(rpipe); + goto fail; + } rpipe->pipe_state |= PIPE_DIRECTOK; wpipe->pipe_state |= PIPE_DIRECTOK; + return (0); + +fail: + knlist_destroy(&rpipe->pipe_sel.si_note); + knlist_destroy(&wpipe->pipe_sel.si_note); +#ifdef MAC + mac_pipe_destroy(pp); +#endif + return (error); } -void +int pipe_named_ctor(struct pipe **ppipe, struct thread *td) { struct pipepair *pp; + int error; - pipe_paircreate(td, &pp); + error = pipe_paircreate(td, &pp); + if (error != 0) + return (error); pp->pp_rpipe.pipe_state |= PIPE_NAMED; *ppipe = &pp->pp_rpipe; + return (0); } void @@ -402,7 +425,9 @@ kern_pipe(struct thread *td, int fildes[2], int flags, struct pipepair *pp; int fd, fflags, error; - pipe_paircreate(td, &pp); + error = pipe_paircreate(td, &pp); + if (error != 0) + return (error); rpipe = &pp->pp_rpipe; wpipe = &pp->pp_wpipe; error = falloc_caps(td, &rf, &fd, flags, fcaps1); @@ -616,25 +641,16 @@ pipeselwakeup(struct pipe *cpipe) * Initialize and allocate VM and memory for pipe. The structure * will start out zero'd from the ctor, so we just manage the kmem. */ -static void -pipe_create(struct pipe *pipe, int backing) +static int +pipe_create(struct pipe *pipe, bool large_backing) { + int error; - if (backing) { - /* - * Note that these functions can fail if pipe map is exhausted - * (as a result of too many pipes created), but we ignore the - * error as it is not fatal and could be provoked by - * unprivileged users. The only consequence is worse performance - * with given pipe. - */ - if (amountpipekva > maxpipekva / 2) - (void)pipespace_new(pipe, SMALL_PIPE_SIZE); - else - (void)pipespace_new(pipe, PIPE_SIZE); - } - - pipe->pipe_ino = alloc_unr64(&pipeino_unr); + error = pipespace_new(pipe, !large_backing || amountpipekva > + maxpipekva / 2 ? SMALL_PIPE_SIZE : PIPE_SIZE); + if (error == 0) + pipe->pipe_ino = alloc_unr64(&pipeino_unr); + return (error); } /* ARGSUSED */ @@ -1068,17 +1084,7 @@ pipe_write(struct file *fp, struct uio *uio, struct uc pipespace(wpipe, desiredsize); PIPE_LOCK(wpipe); } - if (wpipe->pipe_buffer.size == 0) { - /* - * This can only happen for reverse direction use of pipes - * in a complete OOM situation. - */ - error = ENOMEM; - --wpipe->pipe_busy; - pipeunlock(wpipe); - PIPE_UNLOCK(wpipe); - return (error); - } + MPASS(wpipe->pipe_buffer.size != 0); pipeunlock(wpipe); Modified: head/sys/sys/pipe.h ============================================================================== --- head/sys/sys/pipe.h Mon Mar 9 21:01:22 2020 (r358824) +++ head/sys/sys/pipe.h Mon Mar 9 21:55:26 2020 (r358825) @@ -142,6 +142,6 @@ struct pipepair { #define PIPE_LOCK_ASSERT(pipe, type) mtx_assert(PIPE_MTX(pipe), (type)) void pipe_dtor(struct pipe *dpipe); -void pipe_named_ctor(struct pipe **ppipe, struct thread *td); +int pipe_named_ctor(struct pipe **ppipe, struct thread *td); void pipeselwakeup(struct pipe *cpipe); #endif /* !_SYS_PIPE_H_ */ _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
