From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f178.google.com (mail-qk1-f178.google.com [209.85.222.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 14A7934F492 for ; Wed, 3 Dec 2025 16:08:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764778094; cv=none; b=BrYaSw+n1N6nZJovKr4f99o0BF1CKrGoxpjLtwaFkYyoifSI0qUfIbbTvDvCNNxdQf7K51v4A/BhSRJ2Tq9qid3KckX/vzngyu5nDSxWjxdVaq6mN90jTpD2v5OY554ziVJuQ1j0gXwlyrRcYL4I6Wyc5LT8ud/HcdrmhQ7fcHU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764778094; c=relaxed/simple; bh=t8UdtXjAc59D4J6GvJTeXGoV+4cjmIdKxrqekSmeSRA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=e56b5JNKC72/YwpH2DZGnCH0ZHhFV8u7IOAxPePwP4u7yJeqQtAB6eul9tma2WL43FGjtL2WRDIZBlWO6Zw2V+rDQSYvpF08QUS9Ee7YDU8nWpygQgAG82MsNUqINj6WfuMNn4CF7WwwmWnxqmEtMTlHYKG5RcSWrzprgHa9gJs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=dFfvKWWY; arc=none smtp.client-ip=209.85.222.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dFfvKWWY" Received: by mail-qk1-f178.google.com with SMTP id af79cd13be357-8b144ec3aa8so636249685a.2 for ; Wed, 03 Dec 2025 08:08:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764778092; x=1765382892; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=vjK1XAu2xT25O+AavGRfhzxz/zLiQnbfHyTquGAL0Ms=; b=dFfvKWWYFDmjSYj1bgic3CbMIl+7PQ9pzh01xlPUlw1Lq/3hsAvvtVOLGuGIyEjYJO eASOAIdCdVFV0buL93CzmAsC9nmm2NeOGTLBe8DdF4yXPeyRvRrf+5niEpfX5CK1EpP8 nB9MkKFvBb5aAwdckyK4EM71mm8TrQcB0VRGe98Hqp1e7+pLOWaq8DD0mZjly70bYonB I8eFi8pG46sWv6TxT7RJFCw7mMJj9vJ0bxEErO7A8MZ0Iya1SvDYDSDWL2tU5/JCcOvn UsDHyXuSqrmLzpWrnB+kiWP8ykfla7tJTt6rE9UieHVSksLzsSRKawUVDLWwcMOaWJZt iH3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764778092; x=1765382892; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=vjK1XAu2xT25O+AavGRfhzxz/zLiQnbfHyTquGAL0Ms=; b=mbta67x1wRT/brb/i5avqzvT4mK5doolvLb+02dyjOspnvzs1pL+i3WKI1U6b1C7N+ kl82+LaJB7qMHOfLA9OGOLu/A1hQ9viCIPGDeQdLQQZFMUg0kJaIOobL29TDKVQsO548 UV/Sg0IfjXo05Gcdj28JmamVPsZ5+dmLhWYp5Hl/LCoveMOPuhH3o3kl8aZAvw0ulHp5 GuOZoYPzcey4hN/YarGGO6He5fIEIZVUkCSPUOK22lmh9XUg5tAMSOHV8i4Xentevkdr BCFRODVKfOwBG+uqnl5znvLYqRkskNOIUZWYTmbHQZ9I6f5TA9YFBhth+ZYkDhANnpGh CA7A== X-Forwarded-Encrypted: i=1; AJvYcCWlph5fjFkPtVVTW2wB1/+KejC5q0bQTZDECTUUuLYP8+K6JBdXBkjW7cao0W0dC35linVEzECh8B7dqrc=@vger.kernel.org X-Gm-Message-State: AOJu0YwjesUHKLhMkFLLlbF/pDcXYHKVQB24EvTkMoKiNf2vWMdwd3RN 6Htq6TvYrghxPqztW3Cj4qMy7HCIqdnpKnRVtDzrQGGn8XrFcAvYMyrB X-Gm-Gg: ASbGnctgE/t8iXL7cWWvvLsyklVsT9M3Bxp5g3yQsjfr+6y2Y4aRNJeOtCzwlSwQ6Y0 soFK+TRd+GWQeB7vfPCaDNrkMjk2i+UL0wWDf49aQ5eiGE1H66TUnHmUmypOvdS9AQxL9C0y15j PBaMHqNOUqlccRu5I+Y/HYH+vgnlo/EtzIlpq4Ae3StYFJSoeD+iJwhYr6Nc4U4+aiNYx2PyD+n eG99DVfN66bDbNqfHb1f3oxqNs7p7KcAXvCq8Qn1IzWr70nWEE3cVDbEBCMIXSufYbZF1W49pVu Ti3IbuO+YHcYpPDy+PdIfR1Ru3juIYpi726JIUqI5TsqjAmqQ5cTOTIngc9l1ADS73itP1+Lgxh 9uLUBkj+SknyDx4YUwcWGedrQkaZ1l7uK6E/XgCn0NtpuzrYCZTuZ5lM6z3A41VMrDu6DykS8yE 8uMVtSFiDE/OKSc2ppW4bafhqOuplte0ByGdIzNA== X-Google-Smtp-Source: AGHT+IG8XNHFYN5A+3Cg7wEkan0iO0paJZc5a0d8DfhoogQB7q95o21uPOmzpwu8og6VMUZxzU+/rA== X-Received: by 2002:a05:620a:c55:b0:8b2:e38d:2f03 with SMTP id af79cd13be357-8b5e47d0321mr308095785a.9.1764778091859; Wed, 03 Dec 2025 08:08:11 -0800 (PST) Received: from [192.168.0.155] ([170.10.253.128]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8b5299a6fdcsm1324637585a.20.2025.12.03.08.08.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 03 Dec 2025 08:08:11 -0800 (PST) Message-ID: Date: Wed, 3 Dec 2025 11:08:10 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] ext2: factor out ext2_fill_super() teardown path To: Jan Kara Cc: jack@suse.com, linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org References: <20251203045048.2463502-1-vivek.balachandhar@gmail.com> Content-Language: en-CA From: Vivek BalachandharTN In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit thanks for the review. You are right — the helper has only one call site and bh is uninitialized on some failed_sbi paths. I will drop this patch and look for a more meaningful cleanup in ext2. Vivek On 2025-12-03 5:40 a.m., Jan Kara wrote: > On Wed 03-12-25 04:50:48, Vivek BalachandharTN wrote: >> The error path at the end of ext2_fill_super() open-codes the final >> teardown of the ext2_sb_info structure and associated resources. >> Centralize this into a small helper to make the control flow a bit >> clearer and avoid repeating the same cleanup sequence in multiple >> labels. >> >> Behavior is unchanged. >> >> Signed-off-by: Vivek BalachandharTN > This is pointless - no point in factoring out helper when it has a single > call site. Also your patch is broken in several ways (both in correctness > and style). Please be more thoughtful when submitting patches. > > Honza > >> +static void ext2_free_sbi(struct super_block *sb, >> + struct ext2_sb_info *sbi, >> + struct buffer_head *bh) >> +{ >> + if (bh) >> + brelse(bh); >> + >> + fs_put_dax(sbi->s_daxdev, NULL); >> + sb->s_fs_info = NULL; >> + kfree(sbi->s_blockgroup_lock); >> + kfree(sbi); >> +} >> + >> static int ext2_fill_super(struct super_block *sb, struct fs_context *fc) >> { >> struct ext2_fs_context *ctx = fc->fs_private; >> @@ -1251,12 +1264,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc) >> kvfree(sbi->s_group_desc); >> kfree(sbi->s_debts); >> failed_mount: >> - brelse(bh); >> failed_sbi: >> - fs_put_dax(sbi->s_daxdev, NULL); >> - sb->s_fs_info = NULL; >> - kfree(sbi->s_blockgroup_lock); >> - kfree(sbi); >> + ext2_free_sbi(sb, sbi, bh); >> return ret; >> } >> >> -- >> 2.34.1 >>