From nobody Mon Sep 22 17:42:34 2025 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 4cVr6M5nyQz68CP7; Mon, 22 Sep 2025 17:42:35 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R13" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4cVr6M52ksz3sWV; Mon, 22 Sep 2025 17:42:35 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758562955; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Wk7D1PUDgYRzy/BzlPgjC6lzskMa48u4mPTN1oE5WnQ=; b=cn4f1U+CJldvvlTTj7zZ9P3M2yLZAwfHkPXlDVGCrok2yvwR1lNL/UM85KGvUPReuk8d+l MqKf6o8Fs/+fHyokYQw0FUUwPoGXTYiPXWGiGMm4VR+3vP8qH4XKtFwbZBWDI/hN7lctoj D223MPI8lFw5/C2c2VLxUGlpvgzdzW9Jec+O9IUWC7L7yaj9X8rDRuuJryItLxtHJLzpFp ya8fmE1PRE+NNj8ucupMs8DW6TYtByj+96xlT2FIZlxwGBRHhs2WRVefgtHnXgtWEnEb5e LHJHhSVqCmWTKWn5Qdv4hiZcpXso6lgQZLTEFtRBsA0FJbITV1mQrsB0FgpXNw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1758562955; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Wk7D1PUDgYRzy/BzlPgjC6lzskMa48u4mPTN1oE5WnQ=; b=evlII62VYQ7mQQqJpU9/+2cbw2L7EfeGMEng+Ii91a+HxZR6KHO2T/3W5uGQYnBP3rwTPs dJStQzNpl56c3LrYHspnlSz//cDqjldhyq0v3DCMnte4r73X8hPq4sV74Lxhz8Vr9Bju97 h/YG9/pd2UCKog131HYzUZpvruKwCbBQ0KkY1eRxM9dfUzwT4gLwg5JEETxlJsPX6mgt1e 0MsFkQEj1Ta1vLSk7VH+Zhv765o5i4/m//UEKpYRunR+6XSOGeA1qFTAkxVKGNSBdVGlJa nWC5dSxIj0Q4yWHStz1g3m1NnfVyVO4ZExWsVG60Ozl3WVtfIhIl4ym/Q/eX3w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1758562955; a=rsa-sha256; cv=none; b=vPVUMx2wCAdzeSIWpViNIswbQsUCn5PZ4MkiIGCcq8QbfIYZRv4V1nAdLbYqoHKQDKYXoS fcWOvVAXjrBSmiEali9fXTR63Pt9g4bYEkfSALxG/uIaCsHXOAY1ySAQksfmy5O/H1ymP/ Qb6qnFv3DSe2x4zaYxwox3SX3+Fec30nMmc2GnY3H/dpm/FovmQNuKTJwwMFD5ZhLtHakZ lC12McizBr4bTknKAA6x7QjH1bMB6nVsuVeLE8Ii6uFeYUi5fJviuUifpen3elQ8HMuzmc mT7kZLjIORkGunIAJnhEzQvfvJIOtM0nMCH+kor6TdOo0fLru4IipId21L8CxQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from [10.103.41.71] (unknown [193.115.216.57]) (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 did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4cVr6M0zRZzxxr; Mon, 22 Sep 2025 17:42:35 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Mon, 22 Sep 2025 18:42:34 +0100 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: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: 40a42785dbba - main - fcntl(F_SETFL): only allow one thread to perform F_SETFL Content-Language: en-US To: Mateusz Guzik Cc: Konstantin Belousov , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202509191419.58JEJsvj031867@gitrepo.freebsd.org> <92831372-745d-4612-b38f-aeb235dd8cca@FreeBSD.org> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 9/22/25 13:40, Mateusz Guzik wrote: > On Mon, Sep 22, 2025 at 7:39 PM John Baldwin wrote: >> >> On 9/22/25 04:54, Mateusz Guzik wrote: >>> On Mon, Sep 22, 2025 at 10:41 AM John Baldwin wrote: >>>> >>>> On 9/19/25 10:19, Konstantin Belousov wrote: >>>>> The branch main has been updated by kib: >>>>> >>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=40a42785dbba93cc5196178fc49d340c1a89cabe >>>>> >>>>> commit 40a42785dbba93cc5196178fc49d340c1a89cabe >>>>> Author: Konstantin Belousov >>>>> AuthorDate: 2025-09-11 10:05:04 +0000 >>>>> Commit: Konstantin Belousov >>>>> CommitDate: 2025-09-19 14:19:13 +0000 >>>>> >>>>> fcntl(F_SETFL): only allow one thread to perform F_SETFL >>>>> >>>>> Use f_vflags file locking for this. >>>>> Allowing more than one thread handling F_SETFL might cause de-sync >>>>> between real driver state and flags. >>>>> >>>>> Reviewed by: markj >>>>> Tested by: pho >>>>> Sponsored by: The FreeBSD Foundation >>>>> MFC after: 2 weeks >>>>> Differential revision: https://reviews.freebsd.org/D52487 >>>> >>>> Thanks for fixing this. I still slightly worry that "home-grown" locks >>>> aren't visible to WITNESS and it's checking. >>>> >>> >>> Another problem with these is that they don't do adaptive spinning. >>> >>> In particular for file offset, it *is* putting threads off cpu in real >>> workloads when it plausibly could be avoided. >>> >>> I think the real thing to do here is to drop the hand-rolled machinery >>> and use an sx lock. >>> >>> Currently struct file is 80 bytes which is a very nasty size from >>> caching standpoint. >>> >>> Locks are 32 bytes in size, which is another problem, but ultimately >>> one can be added here without growing the struct past 128 bytes. >>> >>> The only issue here is that files are marked as NOFREE, so this memory >>> can *never* be reclaimed. >>> >>> One could be tempted to use smr here, but the cost of smr_enter is >>> prohibitive. There is a lazy variant which does not do atomics, which >>> perhaps could work, but that 0 users in the tree and was probably >>> never tested. >>> >>> With 32-bit archs going away I don't think it's a big deal though. >>> >>> For interested, on Linux the struct is 256 bytes. >> >> I had suggested in an earlier review adding an sx-pool similar to our >> existing mtxpool and using that. That would avoid bloating the structure >> with a dedicated lock. >> > > Per my previous e-mail the offset lock is already contested. > > Using a pool over a lock embedded into the struct would hinder performance. > > I explained why I don't consider embedding sx into struct file to be a problem. Fair enough. Certainly simpler. -- John Baldwin