Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754288AbbETWvj (ORCPT ); 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 ); 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" To: Dan Carpenter CC: "open list:STAGING SUBSYSTEM" , Julia Lawall , Adrian Remonda , open list , "Drokin, Oleg" , Greg Donald , Greg Kroah-Hartman , "moderated list:STAGING - LUSTRE..." , Joe Perches 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: 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> <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: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1404 Lines: 49 On 2015/05/20, 1:42 PM, "Dan Carpenter" 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/