From nobody Wed Aug 17 16:21:33 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4M7CvM6fdvz4Zq9N; Wed, 17 Aug 2022 16:21:35 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4M7CvM6FMgz3m4Y; Wed, 17 Aug 2022 16:21:35 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-lf1-x129.google.com with SMTP id w5so19649650lfq.5; Wed, 17 Aug 2022 09:21:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:from:to:cc; bh=yb/FPqGU22/+DSns9kdKQhzCO59S+ITvgr0LQk+IHag=; b=RQmgW1M1cB/KjJeNVYq0V1oOhQ+I47pz096hiBXrD0kZ8tacc+xOoaNsz2Mhux124E 0YOYBsSCEVw++Y2hUHpqMU0Zx0jmtz5H1pUw48B97ucNatBWMx0IBzao6XgtqAhfK4Xr A4Uwg8RyGHSbVqeUVJTrinWfi9A7y5p9+H128RCM13g5FTeKLn/PoRPSLF+9y3N3wUEL 4M5TmiMLGqkpA/mdFD3Tf1kvY7/ifI3nvMFFO5QTG2pTBvCLugwzOqjQrwp8Qk/T1QO1 M9UB6d1GNlbuAD2n3tdL4b0jbozoK9aJ/adB8LOa+FzLartbNR7Opknajx8XQBMJTjK3 SAuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:x-gm-message-state:from:to:cc; bh=yb/FPqGU22/+DSns9kdKQhzCO59S+ITvgr0LQk+IHag=; b=5GPUUZTvsZGlMnLY9Ew+65j9blNGJkFF1izOro82QskRRKx0azDbVp1ffwYqAfNNnm pC2pQBVY4gQ+EkYczIRZTD5i44cbqHm9A2CPRrtiZNRGLJlMa/SwN+yi8hvw5irbeEef 5gcxPch20uhA7vC8Vm00SGIwAFBbb84C6vNmkjmw0fRpAJGvbFtoFlaWQAArIdy5Mm17 HDOvR6WEpo2T7OQ9qbsi1RhGLnv11VXv4uMsaJBahFCKyLoDUNmHPL2fsD/q6O5VIulq I1Oi309Gh4knwt/G6S3Hpc3MGg+EkAla/KU6A+js6rlRAzTDJsKXkJwlhTRYRmY7gJAv kC/g== X-Gm-Message-State: ACgBeo25QcjrMqdHTM/hyP+NSgYFLJyqRpmtDsKFRbuVwP+3mY3kwvOP JoqvbVd732HhOX+yD+UlXWFmS7nOJVg8yjrNQT4egfh9 X-Google-Smtp-Source: AA6agR7NaoUebAzK6JN1eY6p5w8Gv5FQVDErQPv7K7DWwmUx2AJCyWE6iY4pSPyYyLF/hZ+FFkO+GLWXXrF/QlgAexc= X-Received: by 2002:a05:6512:2a89:b0:48b:2556:5d68 with SMTP id dt9-20020a0565122a8900b0048b25565d68mr8611746lfb.53.1660753293835; Wed, 17 Aug 2022 09:21:33 -0700 (PDT) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Received: by 2002:a05:6520:2b8a:b0:1fe:cc4c:a235 with HTTP; Wed, 17 Aug 2022 09:21:33 -0700 (PDT) In-Reply-To: References: <202208171423.27HENpvp024623@gitrepo.freebsd.org> From: Mateusz Guzik Date: Wed, 17 Aug 2022 18:21:33 +0200 Message-ID: Subject: Re: git: 9ac6eda6c6a3 - main - pipe: try to skip locking the pipe if a non-blocking fd is used To: Gleb Smirnoff Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Rspamd-Queue-Id: 4M7CvM6FMgz3m4Y X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On 8/17/22, Gleb Smirnoff wrote: > Mateusz, > > On Wed, Aug 17, 2022 at 02:23:51PM +0000, Mateusz Guzik wrote: > M> + * Try to avoid locking the pipe if we have nothing to do. > M> + * > M> + * There are programs which share one pipe amongst multiple processes > M> + * and perform non-blocking reads in parallel, even if the pipe is > M> + * empty. This in particular is the case with BSD make, which when > M> + * spawned with a high -j number can find itself with over half of the > M> + * calls failing to find anything. > M> + */ > M> + if ((fp->f_flag & FNONBLOCK) != 0 && !mac_pipe_check_read_enabled()) { > M> + if (__predict_false(uio->uio_resid == 0)) > M> + return (0); > M> + if ((atomic_load_short(&rpipe->pipe_state) & PIPE_EOF) != 0) > M> + return (0); > M> + if (atomic_load_int(&rpipe->pipe_buffer.cnt) == 0 && > M> + atomic_load_int(&rpipe->pipe_pages.cnt) == 0) > M> + return (EAGAIN); > M> + } > > While this change definitely can optimize some programs it also can heavily > pessimize others. An application can share pipes and actually expect that > kernel will do proper serialization of writes by different threads. With > this change, the threads that previously were sleeping on the pipe would > enter a busy loop consisting write + kevent/poll syscalls. > > This also breaks API. Although not explicitly narrated by POSIX or our > documentation, but there is a strong contract that, if application had > received a non-blocking fd from event dispatcher (kevent, poll, select) > as writable, then it is possible to write thre non-zero bytes with next > syscall. > > This is a useful behavior, but by no means should be the default. It > should be explicitly enabled by a new fcntl() and make(1) can do that. > I don't understand this whatsoever. I patched the read side, not write. So in particular, should you find there is free space in the pipe to write to, there is literally no change. At worst, if a non-blocking read was racing a write, there is a bigger window where the reader will conclude there is nothing to get and leave. I see the following races: 1. non-blocking reader arrived before the writer managed to busy the pipe, in which case the outcome is the same in both old and new code 2. non-blocking reader arrived after the writer managed to unbusy the pipe, in which case the outcome is once again the same -- the write was completed and reader sees it 3. non-blocking reader arrived after the writer managed to busy the pipe, but before it got unbusied. here the old code would always wait, while the new will or will not wait depending on whether the writer managed to bump data counters or not. Ultimately there is no *new* outcome for any of it. -- Mateusz Guzik