From owner-freebsd-hackers@FreeBSD.ORG Tue Apr 8 13:07:33 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 3381A9A1 for ; Tue, 8 Apr 2014 13:07:33 +0000 (UTC) Received: from mail-wi0-x22b.google.com (mail-wi0-x22b.google.com [IPv6:2a00:1450:400c:c05::22b]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id BBA4A1F7F for ; Tue, 8 Apr 2014 13:07:32 +0000 (UTC) Received: by mail-wi0-f171.google.com with SMTP id q5so6975451wiv.16 for ; Tue, 08 Apr 2014 06:07:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=Blvu/beuisPfGVkEmJIEhp+oV6C8DtoU5LbMZ/bVNwM=; b=jKmm/le0z1o+nbWIUr5eSe3L2BnST6aWrVR3EWNwz5DTbOXsU1+3Ddo5aotmkjfRwR Z4N7MkiN3IB2e3bzE+Hl9tWzIYg+ogBa3O88KTiEsTpQLX+X0IbdM+sUbOyGLQGvZ0gP P9UAeIiH3DZw1D7iN6ih8aiHO7+ih4dX3u+sl2xXd2xPjrsO8ZP1uYu5UFYF9seZTtXP N8chfGEud2aMM5wADOm3dd6nYYdZYIf0vvQwdD7KJwMSBp5IdkpL44V+oeMve8IAQP34 nAlyBz/kx2jGceGNPQU2HV5QA5lc37kfBhbvLTdoHUTcY6AqjABa9Qa/XNv5lhJ9n1Sj 4LAg== X-Received: by 10.181.13.82 with SMTP id ew18mr4512810wid.22.1396962450993; Tue, 08 Apr 2014 06:07:30 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id l10sm2910462wiz.18.2014.04.08.06.07.29 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 08 Apr 2014 06:07:29 -0700 (PDT) Date: Tue, 8 Apr 2014 15:07:27 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: pipe() resource exhaustion Message-ID: <20140408130727.GA11363@dft-labs.eu> Mail-Followup-To: Mateusz Guzik , Konstantin Belousov , Eduardo Morras , freebsd-hackers@freebsd.org References: <20140408130206.e75f3bf6c6df28b6e4839e70@yahoo.es> <20140408121222.GB30326@dft-labs.eu> <20140408123827.GW21331@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140408123827.GW21331@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-hackers@freebsd.org, Eduardo Morras X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Apr 2014 13:07:33 -0000 On Tue, Apr 08, 2014 at 03:38:27PM +0300, Konstantin Belousov wrote: > On Tue, Apr 08, 2014 at 02:12:22PM +0200, Mateusz Guzik wrote: > > That said, supporting a reserve for this one sounds like a good idea and > > not that hard to implement - one can either play with atomics and double > > check or just place a mutex-protected check in pipespace_new (before > > vm_map_find). > > > ... > > I think more reasonable behaviour there is to just fall back to the > buffered pipe if the direct buffer allocation fails. Look at the > pipespace_new() calls in the pipe_create(); probably ignoring the error > would do the trick. Yeah, should have checked the caller. Interesting though how the error was made fatal in thiscase. Anyhow, the following hack following your suggestion indeed makes the issue go away for me: diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c index 6ba52e3..5930cf2 100644 --- a/sys/kern/sys_pipe.c +++ b/sys/kern/sys_pipe.c @@ -647,19 +647,21 @@ pipe_create(pipe, backing) struct pipe *pipe; int backing; { - int error; if (backing) { + /* + * Note that these functions can fail, but we ignore + * the error as it is not fatal and could be provoked + * by users. + */ if (amountpipekva > maxpipekva / 2) - error = pipespace_new(pipe, SMALL_PIPE_SIZE); + (void)pipespace_new(pipe, SMALL_PIPE_SIZE); else - error = pipespace_new(pipe, PIPE_SIZE); - } else { - /* If we're not backing this pipe, no need to do anything. */ - error = 0; + (void)pipespace_new(pipe, PIPE_SIZE); } + pipe->pipe_ino = -1; - return (error); + return (0); } /* ARGSUSED */