Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752853AbcDOGim (ORCPT ); Fri, 15 Apr 2016 02:38:42 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:18585 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751268AbcDOGik (ORCPT ); Fri, 15 Apr 2016 02:38:40 -0400 X-IronPort-AV: E=Sophos;i="5.24,486,1454972400"; d="scan'208";a="214260688" Date: Fri, 15 Apr 2016 08:38:37 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@localhost6.localdomain6 To: Matt Fleming cc: Vaishali Thakkar , linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Saurabh Sengar Subject: Re: [PATCH] efi: Use GFP_ATOMIC instead of GFP_KERNEL In-Reply-To: <20160414214949.GS2829@codeblueprint.co.uk> Message-ID: References: <1460372009-10785-1-git-send-email-vaishali.thakkar@oracle.com> <20160414214949.GS2829@codeblueprint.co.uk> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1812 Lines: 51 On Thu, 14 Apr 2016, Matt Fleming wrote: > On Mon, 11 Apr, at 04:23:29PM, Vaishali Thakkar wrote: > > Function dup_variable_bug is called inside the spinlock. > > This may lead to issues when kzalloc sleeps, so it is > > better to use GFP_ATOMIC in this spinlocked context. > > > > Problem found using Coccinelle. > > Dang it, I broke coccinelle ;) > > > Signed-off-by: Vaishali Thakkar > > --- > > drivers/firmware/efi/vars.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c > > index 0ac594c..d5e2f28 100644 > > --- a/drivers/firmware/efi/vars.c > > +++ b/drivers/firmware/efi/vars.c > > @@ -411,7 +411,7 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid, > > */ > > efivar_wq_enabled = false; > > > > - str8 = kzalloc(len8, GFP_KERNEL); > > + str8 = kzalloc(len8, GFP_ATOMIC); > > if (!str8) > > return; > > > > This was brought up by Saurabh last year, > > https://lkml.kernel.org/r/1446003747-3760-1-git-send-email-saurabh.truth@gmail.com > > dup_variable_bug() is never called while holding the spinlock in > practice, and I'm guessing Coccinelle cannot understand that because > it'd need to look at program control flow, across multiple compilation > units. > > If anyone wants to send a patch to clean up the EFI code so that it's > easier for coccinelle to check it, I'd be happy to review it. I looked at it a bit with Vaishali. I wonder if it would be possible at least to have only one flag? Then one wouldn't have to maintain the subtle relationship between atomic and duplicates. I'm not sure that it would help Coccinelle, but at least one could see more quickly that Coccinelle is giving a false positive. julia