From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757546AbcILTKB (ORCPT ); Mon, 12 Sep 2016 15:10:01 -0400 Received: from smtp13.smtpout.orange.fr ([80.12.242.135]:56290 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756848AbcILTJ4 (ORCPT ); Mon, 12 Sep 2016 15:09:56 -0400 X-ME-Helo: [127.0.0.1] X-ME-Date: Mon, 12 Sep 2016 21:09:53 +0200 X-ME-IP: 92.140.223.119 Newsgroups: gmane.linux.kernel.janitors X-Mozilla-News-Host: news://news.gmane.org:119 From: Christophe JAILLET To: shli@kernel.org, linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Question about commit f9a67b1182e5 ("md/bitmap: clear bitmap if bitmap_create failed"). Message-ID: <752ab1d9-412a-149b-a241-e604040ebaff@wanadoo.fr> Date: Mon, 12 Sep 2016 21:09:48 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Antivirus: avast! (VPS 160912-0, 12/09/2016), Outbound message X-Antivirus-Status: Clean Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, I'm puzzled by commit f9a67b1182e5 ("md/bitmap: clear bitmap if bitmap_create failed"). Part of the commit is: @@ -1865,8 +1866,10 @@ int bitmap_copy_from_slot(struct mddev *mddev, int slot, struct bitmap_counts *counts; struct bitmap *bitmap = bitmap_create(mddev, slot); - if (IS_ERR(bitmap)) + if (IS_ERR(bitmap)) { + bitmap_free(bitmap); return PTR_ERR(bitmap); + } but if 'bitmap' is an error, I think that bad things will happen in 'bitmap_free()' when, at the beginning of the function, we will execute: if (bitmap->sysfs_can_clear) <----------------- sysfs_put(bitmap->sysfs_can_clear); However, the commit log message is really explicit and adding this call to 'bitmap_free' has really been done one purpose. ("If bitmap_create returns an error, we need to call either bitmap_destroy or bitmap_free to do clean up, ...") It is also not consistent with the comment before function bitmap_create(): * if this returns an error, bitmap_destroy must be called to do clean up * once mddev->bitmap is set I may have missed something, but I don't see what. Is this commit correct? Best regards, CJ