From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id 201A2395C060 for ; Thu, 19 May 2022 16:57:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 201A2395C060 Received: by mail-ot1-x336.google.com with SMTP id r3-20020a9d5cc3000000b0060ae1789875so1578370oti.13 for ; Thu, 19 May 2022 09:57:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=7xIFS3PDsaUNx/dIu2FpvTSBUn/N+7ql9CYE6eHzwFs=; b=ZrI1FHCqlTIMvZ1iJ4tXkfq7oYK6xVd03KJ3oDAFbyVJO5e1TCGlSTRx3Bvg/iHAud hsRuouHFHR2uGc3GxUIDYMYW/sMSrvGY9ijrK5S03KvOS5j1JeYeEYSSdlqrW8BRlzjK U69DZrJblJg29eVsK21FJvPg0fByk5H2WWhYHI7gQXPqY9b0XotFy2epVvmGCYAnh9Wk zDlyiLaBq5Hrh0KtLGmRmk1xysSqCJg7wrBy/rw9DJjVt19gQCI9/I7E6i8VN+Kv2PgN nCgXM9B0q9QMcS9VgEGsk29Us260pAqUGCCMgukHFZ7fyCH9V6BrWYmfiNJAuSyRuWJ9 Tnog== X-Gm-Message-State: AOAM531oQ6yhlbuNNM7SGAjlbBwQ8bD+s4iZE+aVPdb4pEBPnDq/OxWS +WMknp+5SFOJ/fmFTD7M/ggc+e2JKFTjXA== X-Google-Smtp-Source: ABdhPJzmva4HSZdGFv1oJBSHHvfUCzarh8uiZ72tXePdJbCT1MkZJbAMcaUI6h2s4T/6NcDVO9iRQQ== X-Received: by 2002:a9d:6e88:0:b0:606:b593:4851 with SMTP id a8-20020a9d6e88000000b00606b5934851mr2262238otr.289.1652979425270; Thu, 19 May 2022 09:57:05 -0700 (PDT) Received: from ?IPV6:2804:431:c7cb:cdd6:8dac:493f:c77a:4af8? ([2804:431:c7cb:cdd6:8dac:493f:c77a:4af8]) by smtp.gmail.com with ESMTPSA id l6-20020a056871068600b000f1c2847f8csm66625oao.32.2022.05.19.09.57.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 May 2022 09:57:04 -0700 (PDT) Message-ID: Date: Thu, 19 May 2022 13:57:02 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH] Avoid RMW of flags2 outside lock (BZ #27842) Content-Language: en-US To: Wilco Dijkstra Cc: 'GNU C Library' References: <7d120257-c779-ee56-932d-17738067f0ea@linaro.org> From: Adhemerval Zanella In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 May 2022 16:57:08 -0000 On 19/05/2022 13:26, Wilco Dijkstra wrote: > Hi Adhemerval, >   >> I don't think this is correct because if the caller issues pthread_create >> after flockfile, funlockfile will not issues the correct operations.  I > > No, the idea of switching off the single-threaded optimization before the lock is > precisely to ensure that you never could get that situation. Note that neither of > the locks in flockfile and funlockfile use _IO_FLAGS2_NEED_LOCK currently, so > this is just being extremely conservative - in principle we could remove the > update or move it after the lock. I am trying to see why exactly we need to disable single-thread optimization on flockfile, since there is no FILE operation that takes a callback where pthread_create might be called beween _IO_acquire_lock. Can't we just remove the _IO_FLAGS2_NEED_LOCK set on flockfile? > >> have a fix that uses a different locking mechanism where the _IO_FLAGS2_NEED_LOCK >> is removed by moving both the thread id and single-thread optimization to the >> locks itself (on Linux tid has at maximum 30-bits, we can use 1 bits for the >> single-thread optimization and 1 bits for congestion optimization). > > Right so you mean moving NEED_LOCK bit into the lock variable? Yes, and making the lock smaller on linux (just a word plus the recursive counter). > >> I would say that with currency scheme where _IO_FLAGS2_NEED_LOCK is stick, >> this is a benign data race (although still undesirable). > > You mean as it is now? It is a real bug since various functions update flags2 > behind a lock, so it is possible for this RMW to cause corruption (but only if > you are already multithreaded, which the update is pointless anyway and we > can just skip it). I don't think it would be possible to corrupt because once pthread_create is called, _IO_FLAGS2_NEED_LOCK will be always set (so RMW won't see a __flags2 without _IO_FLAGS2_NEED_LOCK being set).