From mboxrd@z Thu Jan 1 00:00:00 1970 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 D3D8637D11D for ; Fri, 20 Mar 2026 09:51:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774000265; cv=none; b=eIfhsYHMmdh9giY0LDgVqxdOd9uaOfDp99CnP0t6PMv4tud1Ti0xNov2CGuU2U50uHMtde/lW7ukFF3axcNxtNFNOM2cWIjsElzN6inVVWnpPmXGrCUClda51hFu9FwBJhKR1xnTEbUnfHlYeJEH1hptxnfOchODI4lXUDQSS2M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774000265; c=relaxed/simple; bh=81ED6+CgUpFcKBNy6ByLtr+aptKTAvmkk3/Ukn0gmBc=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=JENEArywTyI9x74hlMtKkgagjBuFQMN2IvleZ+aGCZZeGHRMX7kfRT6y+1wXKoKnjv1nH716jYGJKNYznOSOdA44IggIz+V2AbcMHVtlZiZDtDL0Ap65VkLGDrRMhhskyTuWMZWTc4j2vIx59D62lfcjWbZK0Fa9il9kvJ1azi8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=rMPsxpgs; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="rMPsxpgs" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88CCEC4CEF7; Fri, 20 Mar 2026 09:51:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774000265; bh=81ED6+CgUpFcKBNy6ByLtr+aptKTAvmkk3/Ukn0gmBc=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=rMPsxpgsCBHeuSgg8qY2Nz5pukutxWaiDYGQcI8hnuc4CKW3hbM/84G7qvfdfQVP5 pVDoYK8fMY4hZg24c21XfgYxEpgM27VBOp0wIiwgHXcAPfzVDGAY38TPXXTXBWIeij JBgDhp+/ACVZmcXT02rVd+58+yXvzL8bzNXrSPf/rhm472rkuzJdVBvNkBnA612yLW nHq3889haUH144daGOm/be8BWQRv43ij4lAF9lAZ0WOWyojVZ9JyqElQErYU53a3d+ NqEA8dipIZJo++E0/yfqpjJyYntl5/lAUK2kWV6Kwg3Gi5GWg7HyPz0uVazhKZOG5b UJ5nlmiSOWKLg== From: Pratyush Yadav To: Chenghao Duan Cc: pasha.tatashin@soleen.com, rppt@kernel.org, pratyush@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, jianghaoran@kylinos.cn Subject: Re: [PATCH v1 3/3] mm/memfd_luo: use i_size_write() to set inode size during retrieve In-Reply-To: <20260319012845.29570-4-duanchenghao@kylinos.cn> (Chenghao Duan's message of "Thu, 19 Mar 2026 09:28:45 +0800") References: <20260319012845.29570-1-duanchenghao@kylinos.cn> <20260319012845.29570-4-duanchenghao@kylinos.cn> Date: Fri, 20 Mar 2026 09:51:01 +0000 Message-ID: <2vxzqzpebzi2.fsf@kernel.org> User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Thu, Mar 19 2026, Chenghao Duan wrote: > Use i_size_write() instead of directly assigning to inode->i_size > when restoring the memfd size in memfd_luo_retrieve(). The commit message can be improved. It only explains _what_ the patch does. Readers can see that by looking at the code. So it just repeats information that is already there. To be fair, for more complex patches explaining the what does make sense since it might not always be obvious. But what is almost always be a lot more useful is to explain _why_ this change is made. I intentionally assigned i_size directly here. The reason for that being that no one has access to the inode yet so there is no need for the smp_store_release() since there won't be racy accesses. So my first reaction on reading this was to check if I missed some sort of race condition. I don't see any, but this is exactly the kind of thing the commit message should say. So please, explain why you made this change. The reason can be as simple as "for consistency", but there should be one so reviewers aren't left guessing. > > No functional change intended. > > Signed-off-by: Chenghao Duan > --- > mm/memfd_luo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c > index 413df8c75c1d..5e5971f25c68 100644 > --- a/mm/memfd_luo.c > +++ b/mm/memfd_luo.c > @@ -500,7 +500,7 @@ static int memfd_luo_retrieve(struct liveupdate_file_op_args *args) > } > > vfs_setpos(file, ser->pos, MAX_LFS_FILESIZE); > - file->f_inode->i_size = ser->size; > + i_size_write(file_inode(file), ser->size); For the code change, I am neutral. I don't suppose it makes much of a difference, but if people think this is cleaner fine by me. > > if (ser->nr_folios) { > folios_ser = kho_restore_vmalloc(&ser->folios); -- Regards, Pratyush Yadav