From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D8BF8C433E2 for ; Wed, 16 Sep 2020 18:48:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 84EBD20732 for ; Wed, 16 Sep 2020 18:48:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600282115; bh=RnZKsHhiLjth3I3bq3AMzUzM16OB7O2cGvnpJTlxo7o=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=GfxPTfFunQFd9uMBnhEk7gUONnab0g4t6B3pl6ItWgNM1G9pMQm49cyOicKmF9Lz+ KRTsOb7oVwcJzaFxSHf2K5ZH091ssrJ0h2o8+j2Tw9Tc8eEc/O1Xl7Zldw1j/AoGV8 XWIx9hKDGLU/c4/l2Ewpyqsag8zrtjAMYpVQZgmI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728185AbgIPSsA (ORCPT ); Wed, 16 Sep 2020 14:48:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728156AbgIPSrn (ORCPT ); Wed, 16 Sep 2020 14:47:43 -0400 Received: from mail-lf1-x144.google.com (mail-lf1-x144.google.com [IPv6:2a00:1450:4864:20::144]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E035C06174A for ; Wed, 16 Sep 2020 11:47:43 -0700 (PDT) Received: by mail-lf1-x144.google.com with SMTP id y2so8042098lfy.10 for ; Wed, 16 Sep 2020 11:47:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FvU3RmnxF4Fi3PPUl/NAHaQ6sckA0cZltUifXE0sQUY=; b=AiGhVJYFwlZ76nh2p80nDW5KlWkzhVstC+ekxJgSj3hh4Dl41ChGlQj+8UxI5F9Xc9 pKW8C5G/1nb+f2eUFPyOHFNx3MYZgioEwfLY2TkTz0uC4umYfzyb0loAbP9D1r4fVoeg jK8KF5sV9qMp0+hOp7K8kAkTBJPF+XELbvJLo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FvU3RmnxF4Fi3PPUl/NAHaQ6sckA0cZltUifXE0sQUY=; b=jn8Gg+e+bSM/UPOqxSQXHXZABW1raNGGQapJxpJL9XnwX66+QO2Sl/hydUn3TPHlPU 6C56bLpMp/TtAwklfmEKSGB/uzHykFFw2VM3238a4R36MzEe+7GjArRQZHwt87cOubqN CEkyXm6PnEdg8IttYgnJAL2Mh2LRb1mRGi3gO0AVhYHRFS3INVgkc7/RH4BNAS6pmWhd RD4Is/y8GOXl3LFLnk6ZbUwauojteuseKin2OyqnPZ5JSBfn+cgDCaBnvU0SozTM6kIf 1OeEmcqaMmN0ZAu+AQn23Q2Q+ED4D0QnTr/uVU1O24v/kn8AMl+IDHZiRBjA+4X9I2od YA/Q== X-Gm-Message-State: AOAM533zCQA/rRpua6mkqutRqnCJMpjeecQvPbl4qETgTpYijMT/+3G3 iX7riPYBzbS7mOv3BAgD5mceDC9WyOIUqg== X-Google-Smtp-Source: ABdhPJxpMJ8G63M/ixpN0wqbkUkoekZBrT3iLbKL+Q80em694xWWkyaeEDJW8z0l8RpW/M2T7WtL5A== X-Received: by 2002:a19:4211:: with SMTP id p17mr8936668lfa.480.1600282060717; Wed, 16 Sep 2020 11:47:40 -0700 (PDT) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com. [209.85.167.52]) by smtp.gmail.com with ESMTPSA id n20sm5110535lfh.1.2020.09.16.11.47.38 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 16 Sep 2020 11:47:39 -0700 (PDT) Received: by mail-lf1-f52.google.com with SMTP id x77so8132573lfa.0 for ; Wed, 16 Sep 2020 11:47:38 -0700 (PDT) X-Received: by 2002:a19:4186:: with SMTP id o128mr7813977lfa.148.1600282058590; Wed, 16 Sep 2020 11:47:38 -0700 (PDT) MIME-Version: 1.0 References: <9550725a-2d3f-fa35-1410-cae912e128b9@tessares.net> <37989469-f88c-199b-d779-ed41bc65fe56@tessares.net> <20200916103446.GB3607@quack2.suse.cz> In-Reply-To: <20200916103446.GB3607@quack2.suse.cz> From: Linus Torvalds Date: Wed, 16 Sep 2020 11:47:22 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: Kernel Benchmarking To: Jan Kara Cc: Matthieu Baerts , Michael Larabel , Matthew Wilcox , Amir Goldstein , "Ted Ts'o" , Andreas Dilger , Ext4 Developers List , linux-fsdevel Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Sep 16, 2020 at 3:34 AM Jan Kara wrote: > > wait_on_page_bit_common() has: > > spin_lock_irq(&q->lock); > SetPageWaiters(page); > if (!trylock_page_bit_common(page, bit_nr, wait)) > - which expands to: > ( > if (wait->flags & WQ_FLAG_EXCLUSIVE) { > if (test_and_set_bit(bit_nr, &page->flags)) > return false; > } else if (test_bit(bit_nr, &page->flags)) > return false; > ) > > __add_wait_queue_entry_tail(q, wait); > spin_unlock_irq(&q->lock); > > Now the suspicious thing is the ordering here. What prevents the compiler > (or the CPU for that matter) from reordering SetPageWaiters() call behind > the __add_wait_queue_entry_tail() call? I know SetPageWaiters() and > test_and_set_bit() operate on the same long but is it really guaranteed > something doesn't reorder these? I agree that we might want to make this much more specific, but no, those things can't be re-ordered. Part of it is very much that memory ordering is only about two different locations - accessing the *same* location is always ordered, even on weakly ordered CPU's. And the compiler can't re-order them either, we very much make test_and_set_bit() have the proper barriers. We'd be very screwed if a "set_bit()" could pass a "test_and_set_bit". That said, the PageWaiters bit is obviously very much by design in the same word as the bit we're testing and setting - because the whole point is that we can then at clear time check the PageWaiters bit atomically with the bit we're clearing. So this code optimally shouldn't use separate operations for those bits at all. It would be better to just have one atomic sequence with a cmpxchg that does both at the same time. So I agree that sequence isn't wonderful. But no, I don't think this is the bug. And as you mention, Matthieu sees it on UP, so memory ordering wouldn't have been an issue anyway (and compiler re-ordering would cause all kinds of other problems and break our bit operations entirely). Plus if it was some subtle bug like that, it wouldn't trigger as consistently as it apparently does for Matthieu. Of course, the reason that I don't see how it can trigger at all (I still like my ABBA deadlock scenario, but I don't see anybody holding any crossing locks in Matthieu's list of processes) means that I'm clearly missing something Linus