Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932170AbdIGMdx (ORCPT ); Thu, 7 Sep 2017 08:33:53 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:49992 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932105AbdIGMdw (ORCPT ); Thu, 7 Sep 2017 08:33:52 -0400 Date: Thu, 7 Sep 2017 14:33:49 +0200 From: Greg KH To: Cihangir Akturk Cc: lustre-devel@lists.lustre.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, oleg.drokin@intel.com, andreas.dilger@intel.com Subject: Re: [PATCH] staging: lustre: avoid going through unlock/lock overhead Message-ID: <20170907123349.GB20922@kroah.com> References: <1504781862-15577-1-git-send-email-cakturk@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1504781862-15577-1-git-send-email-cakturk@gmail.com> User-Agent: Mutt/1.9.0 (2017-09-02) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2320 Lines: 70 On Thu, Sep 07, 2017 at 01:57:42PM +0300, Cihangir Akturk wrote: > Unlocking a spin lock and then immediately locking without doing > anything useful in between buys us nothing, except wasting CPU cycles. Not always, it can be a "gate" for other users of the lock. Are you sure that is not what is going on here? Did you test this out on a lustre system? The locks here are anything but trivial... > > Also code size gets smaller. > > Before: > > text data bss dec hex filename > 70415 2356 4108 76879 12c4f drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.o > > After: > > text data bss dec hex filename > 70095 2356 4108 76559 12b0f drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.o > > Signed-off-by: Cihangir Akturk > --- > drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > index 64763aa..5d9cd33 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > @@ -1624,8 +1624,9 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx, > __u64 version; > int rc; > > - again: > +again: > spin_lock(&fps->fps_lock); > +again_locked: > version = fps->fps_version; > list_for_each_entry(fpo, &fps->fps_pool_list, fpo_list) { > fpo->fpo_deadline = cfs_time_shift(IBLND_POOL_DEADLINE); > @@ -1722,10 +1723,8 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx, > } > > /* EAGAIN and ... */ > - if (version != fps->fps_version) { > - spin_unlock(&fps->fps_lock); > - goto again; > - } > + if (version != fps->fps_version) > + goto again_locked; > } > > if (fps->fps_increasing) { > @@ -1754,9 +1753,8 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, struct kib_tx *tx, > } else { > fps->fps_next_retry = cfs_time_shift(IBLND_POOL_RETRY); > } > - spin_unlock(&fps->fps_lock); > > - goto again; > + goto again_locked; Really, gotos backwards? Ick, that's horrid as well, so maybe this is better? I hate this whole codebase... I'll let the Lustre maintainers decide about this one... greg k-h