Use i_size_write() instead of directly assigning to inode->i_size
when restoring the memfd size in memfd_luo_retrieve().

No functional change intended.

Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
---
 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);
 
 	if (ser->nr_folios) {
 		folios_ser = kho_restore_vmalloc(&ser->folios);
-- 
2.25.1

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@kylinos.cn>
> ---
>  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

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@kylinos.cn>
> > ---
> >  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.

On Wed, Mar 18, 2026 at 9:29 PM Chenghao Duan <duanchenghao@kylinos.cn> wrote:
>
> Use i_size_write() instead of directly assigning to inode->i_size
> when restoring the memfd size in memfd_luo_retrieve().
>
> No functional change intended.
>
> Signed-off-by: Chenghao Duan <duanchenghao@kylinos.cn>
> ---
>  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);

Reviewed-by: Pasha Tatashin <pasha.tatashin@soleen.com>

I think, smp_store_release() here is not striclty necessary, but makes
sense to use the right function for consistency.

Thanks,
Pasha