Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752844AbbKYNLm (ORCPT ); Wed, 25 Nov 2015 08:11:42 -0500 Received: from mail-io0-f176.google.com ([209.85.223.176]:33066 "EHLO mail-io0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751675AbbKYNLk convert rfc822-to-8bit (ORCPT ); Wed, 25 Nov 2015 08:11:40 -0500 MIME-Version: 1.0 In-Reply-To: <56559D5C.6090004@lightnvm.io> References: <1448378697-2532-1-git-send-email-ww.tao0320@gmail.com> <5654AE2D.2080802@lightnvm.io> <56559D5C.6090004@lightnvm.io> Date: Wed, 25 Nov 2015 21:11:39 +0800 Message-ID: Subject: Re: [PATCH] lightnvm: missing nvm_lock acquire From: Wenwei Tao To: =?UTF-8?Q?Matias_Bj=C3=B8rling?= Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2611 Lines: 77 Hi Matias Place the lock inside nvm_find_nvm_dev can make code cleaner, but sometime may lack of flexibility, cause redundant lock and unlock, just my opinion. Place the lock inside the function is okay with me, just make sure do the proper up_write(&nvm_lock) in nvm_create_target(), it has a lot 'return'. I just submit a patch set covering the lock issue and the target issue mentioned before, you can have a look before you apply this patch. Thanks. 2015-11-25 19:37 GMT+08:00 Matias Bjørling : > On 11/25/2015 09:57 AM, Wenwei Tao wrote: >> >> Hi Matias >> I think list_for_each_entry_safe(pos, n, head, member) cannot avoid >> race condition >> the item point by ‘n’ can be deleted and freed in the same time we >> operate on 'pos' >> so lock is still necessary. >> > I've made the patch like this, ok? > > To avoid race conditions, traverse dev, media manager, > and target lists and also register, unregister entries > to/from them, should be always under the nvm_lock control. > > Signed-off-by: Wenwei Tao > Signed-off-by: Matias Bjørling > --- > drivers/lightnvm/core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c > index ea6dba5..9f901f6 100644 > --- a/drivers/lightnvm/core.c > +++ b/drivers/lightnvm/core.c > @@ -127,9 +127,11 @@ static struct nvm_dev *nvm_find_nvm_dev(const char > *name) > { > struct nvm_dev *dev; > > + down_write(&nvm_lock); > list_for_each_entry(dev, &nvm_devices, devices) > if (!strcmp(name, dev->name)) > return dev; > + up_write(&nvm_lock); > > return NULL; > } > @@ -388,13 +390,14 @@ static int nvm_create_target(struct nvm_dev *dev, > } > } > > + down_write(&nvm_lock); > > tt = nvm_find_target_type(create->tgttype); > if (!tt) { > pr_err("nvm: target type %s not found\n", create->tgttype); > + up_write(&nvm_lock); > return -EINVAL; > } > > - down_write(&nvm_lock); > list_for_each_entry(t, &dev->online_targets, list) { > if (!strcmp(create->tgtname, t->disk->disk_name)) { > pr_err("nvm: target name already exists.\n"); > -- > 2.1.4 > > -- 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/