Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 5 Dec 2022 16:13:17 +0900
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     Marco Elver <elver@google.com>
Cc:     Naresh Kamboju <naresh.kamboju@linaro.org>,
        rcu <rcu@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        kunit-dev@googlegroups.com, lkft-triage@lists.linaro.org,
        kasan-dev <kasan-dev@googlegroups.com>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Netdev <netdev@vger.kernel.org>,
        Anders Roxell <anders.roxell@linaro.org>
Subject: Re: arm64: allmodconfig: BUG: KCSAN: data-race in p9_client_cb /
 p9_client_rpc
Message-ID: <Y42aDQ0ZOUt4dvYc@codewreck.org>
References: <CA+G9fYsK5WUxs6p9NaE4e3p7ew_+s0SdW0+FnBgiLWdYYOvoMg@mail.gmail.com>
 <CANpmjNOQxZ--jXZdqN3tjKE=sd4X6mV4K-PyY40CMZuoB5vQTg@mail.gmail.com>
 <CA+G9fYs55N3J8TRA557faxvAZSnCTUqnUx+p1GOiCiG+NVfqnw@mail.gmail.com>
 <Y4e3WC4UYtszfFBe@codewreck.org>
 <CA+G9fYuJZ1C3802+uLvqJYMjGged36wyW+G1HZJLzrtmbi1bJA@mail.gmail.com>
 <Y4ttC/qESg7Np9mR@codewreck.org>
 <CANpmjNNcY0LQYDuMS2pG2R3EJ+ed1t7BeWbLK2MNxnzPcD=wZw@mail.gmail.com>
 <Y4vW4CncDucES8m+@codewreck.org>
 <CANpmjNPXhEB6GeMT70UT1e-8zTHf3gY21E3wx-27VjChQ0x2gA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CANpmjNPXhEB6GeMT70UT1e-8zTHf3gY21E3wx-27VjChQ0x2gA@mail.gmail.com>
Precedence: bulk

Marco Elver wrote on Mon, Dec 05, 2022 at 08:00:00AM +0100:
> > Should I just update the wrapped condition, as below?
> >
> > -       err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
> > +       err = wait_event_killable(req->wq,
> > +                                 READ_ONCE(req->status) >= REQ_STATUS_RCVD);
> 
> Yes, this looks good!
> 
> > The writes all are straightforward, there's all the error paths to
> > convert to WRITE_ONCE too but that's not difficult (leaving only the
> > init without such a marker); I'll send a patch when you've confirmed the
> > read looks good.
> > (the other reads are a bit less obvious as some are protected by a lock
> > in trans_fd, which should cover all cases of possible concurrent updates
> > there as far as I can see, but this mixed model is definitely hard to
> > reason with... Well, that's how it was written and I won't ever have time
> > to rewrite any of this. Enough ranting.)
> 
> If the lock-protected accesses indeed are non-racy, they should be
> left unmarked. If some assumption here turns out to be wrong, KCSAN
> would (hopefully) tell us one way or another.

Great, that makes sense.

I've left the commit at home, will submit it tonight -- you and Naresh
will be in Cc from suggested/reported-by tags.

-- 
Dominique