From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752708AbdLLSE5 (ORCPT ); Tue, 12 Dec 2017 13:04:57 -0500 Received: from mail-qt0-f196.google.com ([209.85.216.196]:41743 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751871AbdLLSEw (ORCPT ); Tue, 12 Dec 2017 13:04:52 -0500 X-Google-Smtp-Source: ACJfBov0l67K0y+yJLiCQzhdK9y+vLU/voO+5VSzyojwN8tIeVqLQx8fl/sI/li/2eHFRAvThOjJIA== Date: Tue, 12 Dec 2017 10:04:49 -0800 From: Tejun Heo To: Peter Zijlstra Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org, oleg@redhat.com, kernel-team@fb.com, osandov@fb.com Subject: Re: [PATCH 2/6] blk-mq: replace timeout synchronization with a RCU and generation based scheme Message-ID: <20171212180449.GJ3919388@devbig577.frc2.facebook.com> References: <20171209192525.982030-1-tj@kernel.org> <20171209192525.982030-3-tj@kernel.org> <20171212115657.t6z4l3wilvnzs4ue@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171212115657.t6z4l3wilvnzs4ue@hirez.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Tue, Dec 12, 2017 at 12:56:57PM +0100, Peter Zijlstra wrote: > On Sat, Dec 09, 2017 at 11:25:21AM -0800, Tejun Heo wrote: > > +static inline void blk_mq_rq_update_state(struct request *rq, > > + enum mq_rq_state state) > > +{ > > + u64 new_val = (rq->gstate & ~MQ_RQ_STATE_MASK) | state; > > + > > + if (state == MQ_RQ_IN_FLIGHT) { > > + WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE); > > + new_val += MQ_RQ_GEN_INC; > > + } > > + > > + /* avoid exposing interim values */ > > My paranoia would like to see READ_ONCE() on the rq->gstate load above > as well, that makes it a fully explicit load-store operation. Right now, only the request owner can update the field so there's no data coherency issue but then again there's nothing to lose by adding READ_ONCE there. Will add it. Thanks. -- tejun