From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) (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 1BC312BB13 for ; Thu, 8 Jan 2026 22:45:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767912350; cv=none; b=PuG5WYkwzhYvPheIUkTXtwMp2xsg30Tvy9CnebnamvD0ISnWzK62Zya/bto4ZuCtDLyn/34+rJe8FBb8k9VQGDli0Cgtvx8PiPp4VAOzoimYRGOfqHkQFJbNyJ+bCnok4So+UFpz8a40xwGtIYPBWJcEiyFNA5Uc6jkrySMHKFE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767912350; c=relaxed/simple; bh=MopcDLKLiMoySP+Hcu6mGRks3st/CPqX4gG/IuawNDI=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Camb1ZpCsHF+EYIwfb5MCq845T9YPX7GAxlWPrzsYpb0fE3Q9CpQzj6UBCDyR/1E2AeyiVvcbLwelTUH77uspP2mA67PVQAOvfl3fbpEh2mvuQsrf1rGB6Z+kymnMwyp92q5dEX+8ZkDf8jRAipmqB0ObycBQQ4tVdnlXjbbG1M= 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=GqLaykxU; arc=none smtp.client-ip=209.85.221.49 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="GqLaykxU" Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-431048c4068so1460380f8f.1 for ; Thu, 08 Jan 2026 14:45:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1767912347; x=1768517147; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=v23yl1XRlI6/R87v8YLz/T+/j8Q6Psx2F9P+y+YgX4E=; b=GqLaykxUjJeMmKj3W6BSEGs8gIGCT0yxNLNE7RNzE5CyPJwOz0QsO38ww42Tt/yQEh MDfF4FnKke5NnA7PRMldCqGxbk6MsyXGpDcVO6E0UmHp5Zg6AwEc2UBEn2j3gb2yoiYt 4dBKJUCRiJcSE7vPzVSxechbN6sTvu5C1q2+sCzulrfmRlJe713Pl0jSDTy8af3rWF70 dQkLLFsJJFcfzqKVCavUi0x8V2QlOy32OYDQLj/pwqKYdU870tA4x65rUvp6W3Exx0nh Amj3ss6KpqHxrotSj9/W9LT3jCIvwtdSRtEALVvchndiXMekySBKRvGeko0S6wXh3Es2 MP6Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767912347; x=1768517147; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=v23yl1XRlI6/R87v8YLz/T+/j8Q6Psx2F9P+y+YgX4E=; b=F0WdG0lPAVk6U9FOBjNLtWRr36Jjw7xuXTG3hyZAI41omSsKW9AN/FFE5i1XZ2G87c UGZ8k3EJ1a0Et9Z4i8hJkTZgPpRu2P5NaN+1slmrQUZU8fWS6e7nnn2cT0+C+uIomjGI 3TY4ksD0Wqofio10IlpkDumJklZKfToVquMCV3YvnVYnGbBzDxEwa0OsZysvnggwODOc AmxJMxFEEOCj6Y9Iee7gst/yX3QjVekSrsplYLm+bIXf5gwP/s/i1oA/NdbnkXHQESlm pKhiuR5OSo7lTCr3k7vsdMga87+zMr/4ejiKDGCcy+I4YPZJrMU5WmD/OC9dtQ+2rW8G Ap7Q== X-Forwarded-Encrypted: i=1; AJvYcCVHVc2xlW1tIcgjsZNQ7f1xpkrHOv5m8O+UcTkAEkXCXMjXk3xT7fdcS5BmvDHu6OmXfPYbS1pmaGokSWE=@vger.kernel.org X-Gm-Message-State: AOJu0YwK144MAVNhOokhUxXAxST0FoARtVD0+OI24xR0P625F6JgQyJs aRWS3T4/wEjQx80yqwnxtneWyYBuVSMSinKvCz4O9TWdHvV3qV/Lk0TdKIt/Gg== X-Gm-Gg: AY/fxX7f25vFU2eBuTdgK98WY8OPC9zxCdpXAC7oW5epims09Pkn6btPQeJFW6CsPAC cVcvlMSTdsc1GTiRH33fIrsnCI82P4swpP6uStZWEx9ph02hsr+sm9KH045yYmKWD9KZODTa0wR YdqMHcqxW1Mbs6gYTpd8sOmXet+3N2chuiKZ05nAZtKAfWPOwlD3ZTxA84NNNfYCznU+b+Stgco mwTPyXlCToqjlHeNO+Gias9nlMyhzURpsSDk+P2UGdCEXQLhjoDZ2G/k3SK4ygisAwltznpbC/O tqjSBmytL1F9IhFFQ00jbEdNamOr54Dbgp7ZE3Yedqo6EZPvDsrEKMo6/jAhZnNRBIWJDW3ymmd t3dXjjtWn/3S2EXMZg0D8Urcsa4PYiJThotyhubLoQInXODbWz3W0n6wiaZW3L/vEsaSEKtBtpH N3ynB0lgPRM+eXvGZcgj2Jvn5y/V25eEi+IqsDiXhe/QvgyBljLd2Ae1Prgno4qak= X-Google-Smtp-Source: AGHT+IGzFAB0n5LQiK0CFjp1orx3tC0NUiWOiG2YR2Jt4DpVnrQci46VmyZg1tmpEXWeo4MR0y4q9A== X-Received: by 2002:a05:6000:40dc:b0:431:2ff:128f with SMTP id ffacd0b85a97d-432c362c199mr10613660f8f.6.1767912347243; Thu, 08 Jan 2026 14:45:47 -0800 (PST) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-432bd5ff319sm19030056f8f.43.2026.01.08.14.45.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jan 2026 14:45:46 -0800 (PST) Date: Thu, 8 Jan 2026 22:45:45 +0000 From: David Laight To: Chao Yu Cc: jaegeuk@kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] f2fs: fix error path handling in f2fs_read_data_large_folio() Message-ID: <20260108224545.3019a411@pumpkin> In-Reply-To: <20260107214231.24163-1-chao@kernel.org> References: <20260107214231.24163-1-chao@kernel.org> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 8 Jan 2026 05:42:31 +0800 Chao Yu wrote: > In error path of f2fs_read_data_large_folio(), if bio is valid, it > may submit bio twice, fix it. That isn't the only bug at the bottom of that function. I think I've unravelled the strange loops on the copy in linux-next. The 'goto got_it' could be a normal conditional. The top has: if (rac) folio = readahead_folio(rac); next_folio: if (!folio) goto out: which means you can 'goto out' before setting up a pending 'bio'. I'm sure that could be made a proper loop - although it would cost an indentation. Would certainly be better with only one call to readahead_foilio(), perhaps: next_folio: if (rac) { folio = readahead_folio(rac); if (!folio) goto out: } with just: if (rac) goto next_folio; at the bottom. > > Signed-off-by: Chao Yu > --- > fs/f2fs/data.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index cabaeeb436bd..386d9adfd4bd 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -2568,17 +2568,14 @@ static int f2fs_read_data_large_folio(struct inode *inode, > folio_unlock(folio); > return ret; > } > - > +out: > + f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA); > if (ret) { > - f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA); > - > /* Wait bios and clear uptodate. */ > folio_lock(folio); If I've read the code correctly the 'bio' can contain transfers for a previous folio(s), and might have transfers for this folio, but might not. So relocking the folio may just deadlock. (I've not found the unlock at the end of transfer...) Quite which 'bio' need the flag changed is another question. David > folio_clear_uptodate(folio); > folio_unlock(folio); > } > -out: > - f2fs_submit_read_bio(F2FS_I_SB(inode), bio, DATA); > return ret; > } >