From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755539AbaIEB6R (ORCPT ); Thu, 4 Sep 2014 21:58:17 -0400 Received: from mail-oa0-f52.google.com ([209.85.219.52]:41577 "EHLO mail-oa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751132AbaIEB6P (ORCPT ); Thu, 4 Sep 2014 21:58:15 -0400 Date: Thu, 4 Sep 2014 20:58:10 -0500 From: Chuck Ebbert To: Jens Axboe Cc: Alexander Gordeev , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] blk-mq: Cleanup blk_mq_tag_busy() and blk_mq_tag_idle() Message-ID: <20140904205810.3daf3a87@as> In-Reply-To: <5409122A.7020703@kernel.dk> References: <1860c570e5efa8ece323a7ba20f3597bd5e99d25.1409775956.git.agordeev@redhat.com> <54077B91.5040408@kernel.dk> <20140904202654.27f1d9e0@as> <5409122A.7020703@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 04 Sep 2014 19:30:18 -0600 Jens Axboe wrote: > On 09/04/2014 07:26 PM, Chuck Ebbert wrote: > > On Wed, 03 Sep 2014 14:35:29 -0600 > > Jens Axboe wrote: > > > >> On 09/03/2014 02:33 PM, Alexander Gordeev wrote: > > > >>> diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h > >>> index 6206ed1..795ec3f 100644 > >>> --- a/block/blk-mq-tag.h > >>> +++ b/block/blk-mq-tag.h > >>> @@ -66,23 +66,22 @@ enum { > >>> BLK_MQ_TAG_MAX = BLK_MQ_TAG_FAIL - 1, > >>> }; > >>> > >>> -extern bool __blk_mq_tag_busy(struct blk_mq_hw_ctx *); > >>> +extern void __blk_mq_tag_busy(struct blk_mq_hw_ctx *); > >>> extern void __blk_mq_tag_idle(struct blk_mq_hw_ctx *); > >>> > >>> static inline bool blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx) > >>> { > >>> - if (!(hctx->flags & BLK_MQ_F_TAG_SHARED)) > >>> - return false; > >>> - > >>> - return __blk_mq_tag_busy(hctx); > >>> + if (hctx->flags & BLK_MQ_F_TAG_SHARED) { > >>> + __blk_mq_tag_busy(hctx); > >>> + return true; > >>> + } > >>> + return false; > >>> } > >> > >> The normal/fast path here is the flag NOT being set, which is why > >> it was coded that way to put the fast path inline. > >> > >>> > >>> static inline void blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx) > >>> { > >>> - if (!(hctx->flags & BLK_MQ_F_TAG_SHARED)) > >>> - return; > >>> - > >>> - __blk_mq_tag_idle(hctx); > >>> + if (hctx->flags & BLK_MQ_F_TAG_SHARED) > >>> + __blk_mq_tag_idle(hctx); > >>> } > >> > >> Ditto > > > > Shouldn't it just add unlikely() then? That way it's obvious what > > the common case is, instead of relying on convoluted code. > > It's a common construct. Besides, if you find a flag-not-set check > convoluted, then I hope you are not programming anything I use. > That's a bit of a straw man, imho. > Sure, it's a common construct. But there's nothing there to prevent the optimizer from rearranging things any way it pleases. Nor is there anything keeping a human from doing the same. ;)