Received: by 2002:a05:7412:e794:b0:fa:551:50a7 with SMTP id o20csp2430839rdd; Fri, 12 Jan 2024 09:09:08 -0800 (PST) X-Google-Smtp-Source: AGHT+IEOUSh9c/Jmu24DhhZOCRUXKJOhOgTeqk4saoRgs+2k9B/nRSwUz0RaFXkJS75QBzlg5wN3 X-Received: by 2002:a05:6512:986:b0:50e:9356:86e0 with SMTP id w6-20020a056512098600b0050e935686e0mr687405lft.112.1705079348689; Fri, 12 Jan 2024 09:09:08 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705079348; cv=none; d=google.com; s=arc-20160816; b=vYUoziWQKw//vtOjqV9ZNfuRVPqSosKDo2eC2KAlWSSYjis9P2iMAgn0lUfUfuPFoT rX0DnJfoC5Y/UBa2Y4k44+i5qJ/TOtQAyayva+KXCXGhrQWZGTx/wp2F+nBMdGboMezf sgsbWRnR5T7dJyqKnaCb2cVrTJEUYzdXBres1BllBJDmvgi8HflNp8XKe8PvKplE/Kpy Z5v4FxjeSzNBVvlxHSrPXIIuh5+teKthABwdfx9+uYKH26ux5n5TPtE7YNNtZjHvZ5qc N2iksz913fetIver1vp5KzEUcaKalU6xC/3TQao+tzlTRoMqhAgo2ocYRle/TwdVRGj8 s2hw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date:dkim-signature; bh=KMeHdglnWzxd7RAnUb7Ks2qUqeLYXYvyecYuHzEA9Mg=; fh=9wUqIqrsmgwv6q3rIxJStkj/a26eyje4p25UuYIwgK8=; b=xa2gif0EM+s/7kovcjyM1B34c0cwptlExi7T+Otydx6gWoux/spvFxyreHQC+pc3tK QEDh0PSrMXIm3K3ZwbcosiLYbAzygBvvwKQca6m1opR28obMg6ta3H9aRMpO5ecBU2Kc M6bOenZETP62x5mNtcuOZxwDEURjYcCC1ml18myjvg7p6mzyaWTS6FuvM0n7CdS5//m/ glRhd8CU7FUhL5Hvv8dBChUXeaZ2vqhscw7oKU8AlZd0yFNo5vsAKZ3YiEmfgrL6AEvQ 5h4yB5FE8fgyTfgD/Eyn2YkQkJ50MwDb5fTktADJ4jLgN1NUL6J85YV9W68o8GmVMpTC UsKQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=oT6zVPJ1; spf=pass (google.com: domain of linux-kernel+bounces-24890-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24890-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id y11-20020a1709060a8b00b00a2814cb9d0dsi1555980ejf.659.2024.01.12.09.09.08 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 09:09:08 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-24890-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=oT6zVPJ1; spf=pass (google.com: domain of linux-kernel+bounces-24890-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-24890-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 6E0621F247A3 for ; Fri, 12 Jan 2024 17:09:08 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E08141B5B1; Fri, 12 Jan 2024 17:09:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="oT6zVPJ1" Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 20FE81B5A5 for ; Fri, 12 Jan 2024 17:09:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7A3ABC43390; Fri, 12 Jan 2024 17:09:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1705079340; bh=GDl6p3UZsPjLAqZ6riRDVB5aigv9wH3zVmTnLWIcCLI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oT6zVPJ1R4hpGcq1aYlIz3BQCRh9WApN8ONK8zmQdxhnpvQ4qnEitpsH/OaFXD7Lv pQYWqm+0WDhpeJreemXvrlAuK7AU4Hc4uIa4nlQpz0d9OLm0gGEjjyuSO4bLv48gNO wBpO2MgUt2REUjD9v8uQbuBBLgFkt9lh8NTLS8FdPZ7WLpCskAkiT3zi21d7UqBtVn JS1S7XlfxIALEhlgXbbrghvQDmBraimE2YU1/uOKIyP/EdEnD3lIBU9TDoZp54fiW3 BjRIr4aEZruzFNycr5wKIvzaDW8rkO23ZZAW9/S2AOwa7o5iLpehdKKUeZX6/SHQtS MMuYRYDBwcCZw== Date: Fri, 12 Jan 2024 09:08:58 -0800 From: Jaegeuk Kim To: Al Viro Cc: Linus Torvalds , Linux Kernel Mailing List , Linux F2FS Dev Mailing List Subject: Re: [GIT PULL] f2fs update for 6.8-rc1 Message-ID: References: <20240112071242.GA1674809@ZenIV> 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-Disposition: inline In-Reply-To: <20240112071242.GA1674809@ZenIV> On 01/12, Al Viro wrote: > On Thu, Jan 11, 2024 at 09:05:51PM -0800, Linus Torvalds wrote: > > On Thu, 11 Jan 2024 at 10:28, Jaegeuk Kim wrote: > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/f2fs-for-6.8-rc1 > > > > Hmm. I got a somewhat confusing conflict in f2fs_rename(). > > > > And honestly, I really don't know what the right resolution is. What I > > ended up with was this: > > > > if (old_is_dir) { > > if (old_dir_entry) > > f2fs_set_link(old_inode, old_dir_entry, > > old_dir_page, new_dir); > > else > > f2fs_put_page(old_dir_page, 0); > > Where would you end up with old_dir_page != NULL and old_dir_entry == NULL? > old_dir_page is initialized to NULL and the only place where it's altered > is > old_dir_entry = f2fs_parent_dir(old_inode, &old_dir_page); > Which is immediately followed by > if (!old_dir_entry) { > if (IS_ERR(old_dir_page)) > err = PTR_ERR(old_dir_page); > goto out_old; > } > so we are *not* going to end up at that if (old_is_dir) in that case. It seems [1] changed the condition of getting old_dir_page reference as below, which made f2fs_put_page(old_dir_page, 0) voided. - if (S_ISDIR(old_inode->i_mode)) { + if (old_is_dir && old_dir != new_dir) { old_dir_entry = f2fs_parent_dir(old_inode, &old_dir_page); if (!old_dir_entry) { if (IS_ERR(old_dir_page)) [1] 7deee77b993a ("f2fs: Avoid reading renamed directory if parent does not change") > > Original would have been more clear as > if (old_is_dir) { > if (old_dir != new_dir) { > /* we have .. in old_dir_page/old_dir_entry */ > if (!whiteout) > f2fs_set_link(old_inode, old_dir_entry, > old_dir_page, new_dir); > else > f2fs_put_page(old_dir_page, 0); > } > f2fs_i_links_write(old_dir, false); > } > - it is equivalent to what that code used to do. And "don't update .. > if we are leaving a whiteout behind" was teh bug fixed by commit > in f2fs tree... > > The bottom line: your variant is not broken, but only because > f2fs_put_page() starts with > static inline void f2fs_put_page(struct page *page, int unlock) > { > if (!page) > return; > > IOW, you are doing f2fs_put_page(NULL, 0), which is an explicit no-op.