Re: [PATCH v1 3/3] mm/memfd_luo: use i_size_write() to set inode size during retrieve

[Mike Rapoport]

On Fri, Mar 20, 2026 at 09:51:01AM +0000, Pratyush Yadav wrote:
> 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 <duanchenghao@xxxxxxxxxx>
> > ---
> >  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.

I'd also add a comment here explaining that i_size_write() is for
consistency :)
 
> >  
> >  	if (ser->nr_folios) {
> >  		folios_ser = kho_restore_vmalloc(&ser->folios);
> 
> -- 
> Regards,
> Pratyush Yadav

-- 
Sincerely yours,
Mike.