From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754288AbbETWvj (ORCPT <rfc822;w@1wt.eu>);
	Wed, 20 May 2015 18:51:39 -0400
Received: from mga03.intel.com ([134.134.136.65]:43825 "EHLO mga03.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751774AbbETWvg convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 20 May 2015 18:51:36 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.13,466,1427785200"; 
   d="scan'208";a="574539011"
From: "Dilger, Andreas" <andreas.dilger@intel.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
CC: "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
        Julia Lawall <Julia.Lawall@lip6.fr>,
        Adrian Remonda <adrianremonda@gmail.com>,
        open list <linux-kernel@vger.kernel.org>,
        "Drokin, Oleg" <oleg.drokin@intel.com>,
        Greg Donald <gdonald@gmail.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "moderated list:STAGING - LUSTRE..." <HPDD-discuss@ml01.01.org>,
        Joe Perches <joe@perches.com>
Subject: Re: [PATCH 4/4] Staging: lustre: sparse lock warning fix
Thread-Topic: [PATCH 4/4] Staging: lustre: sparse lock warning fix
Thread-Index: AQHQkZlsfLU5eGQ7J0ymQ7HrCNgz4J2Cs0qAgAJ02gCAAJCRgIAAA7eA///QMoA=
Date: Wed, 20 May 2015 22:51:34 +0000
Message-ID: <D1826B62.F1AAD%andreas.dilger@intel.com>
References: <1431974091-26363-1-git-send-email-adrianremonda@gmail.com>
 <1431974091-26363-2-git-send-email-adrianremonda@gmail.com>
 <1431974091-26363-3-git-send-email-adrianremonda@gmail.com>
 <1431974091-26363-4-git-send-email-adrianremonda@gmail.com>
 <1431974091-26363-5-git-send-email-adrianremonda@gmail.com>
 <20150518212115.GN14154@mwanda> <D18216B5.F1936%andreas.dilger@intel.com>
 <20150520192924.GS22558@mwanda> <20150520194243.GG4150@mwanda>
In-Reply-To: <20150520194243.GG4150@mwanda>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.254.25.204]
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8CB75E212756A9478DAC00B750DCB8B9@intel.com>
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2015/05/20, 1:42 PM, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:

>In Smatch, it the equivalent warning is turned off by default because
>there are too many false positives, but you can enable it with the
>--spammy flag.
>
>kchecker --spammy drivers/staging/lustre/lustre/ptlrpc/nrs.c
>
>drivers/staging/lustre/lustre/ptlrpc/nrs.c:512 nrs_resource_put_safe()
>warn: 'spin_lock:&nrs->nrs_lock' is sometimes locked here and sometimes
>unlocked.

Would this be happier with something like:

        for (i = 0; i < NRS_RES_MAX; i++) {
		if (pols[i] == NULL)
			continue;
        

		if (nrs == NULL) {
			nrs = pols[i]->pol_nrs;
			if (likely(nrs != NULL)) /* make sparse happy */
				spin_lock(&nrs->nrs_lock);
		}
		nrs_policy_put_locked(pols[i]);
	}

	if (nrs != NULL)
	spin_unlock(&nrs->nrs_lock);

so that the "if" conditions are the same?  The code definitely doesn't
have a bug, because the lock is only locked once when nrs is first set,
and only unlocked if it is set.  Or is there a comment to put there that
will quiet the static checker?


Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division