From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753639AbbETTaA (ORCPT ); Wed, 20 May 2015 15:30:00 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:40505 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751818AbbETT37 (ORCPT ); Wed, 20 May 2015 15:29:59 -0400 Date: Wed, 20 May 2015 22:29:25 +0300 From: Dan Carpenter To: "Dilger, Andreas" Cc: Adrian Remonda , "open list:STAGING SUBSYSTEM" , "moderated list:STAGING - LUSTRE..." , Greg Donald , open list , "Drokin, Oleg" , Julia Lawall , Greg Kroah-Hartman , Joe Perches Subject: Re: [PATCH 4/4] Staging: lustre: sparse lock warning fix Message-ID: <20150520192924.GS22558@mwanda> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 20, 2015 at 04:51:59PM +0000, Dilger, Andreas wrote: > On 2015/05/18, 3:21 PM, "Dan Carpenter" wrote: > > >On Mon, May 18, 2015 at 08:34:51PM +0200, Adrian Remonda wrote: > >> Fixed sparse warning: context imbalance in 'nrs_resource_put_safe' - > >> 'different lock contexts for basic block' by releasing the lock on each > >> iteration of the for loop. > >> > > > >That changelog doesn't sound correct at all. That's not a correct > >motivation or explanation. > > > >I reviewed the patch and it's likely going to cause dead locks. The code > >is trying to take the spinlock for the first pointer in the array and > >release it at the end. Now it takes the first pointer's spinlock a > >bunch of times (dead lock) and releases it once (will not happen because > >we are already dead). > > It isn't clear to me what the checkpatch complaint actually means? Is it > that the spin_lock() and spin_unlock() calls have different amounts of > indentation? > It's not a checkpatch.pl warning, it's a Sparse warning. Sparse is crappy at reporting locking bugs. It's mostly false positives. I think it's saying that some paths lock and unlock some don't. Smatch is also fairly crappy at finding locking bugs, unfortunately. I need to re-write it using modern features and cross function analysis. regards, dan carpenter