On Wed, Sep 03, 2025 at 03:52:21PM +0100, Lorenzo Stoakes wrote:
> On Tue, Sep 02, 2025 at 11:08:10AM -0400, Liam R. Howlett wrote:
> >
> > One nit below.
> > > static inline void set_vma_from_desc(struct vm_area_struct *vma,
> > > - struct vm_area_desc *desc)
> > > + struct file *orig_file, struct vm_area_desc *desc)
> > > {
> > > /*
> > > * Since we're invoking .mmap_prepare() despite having a partially
> > > @@ -258,7 +258,13 @@ static inline void set_vma_from_desc(struct vm_area_struct *vma,
> > >
> > > /* Mutable fields. Populated with initial state. */
> > > vma->vm_pgoff = desc->pgoff;
> > > - if (vma->vm_file != desc->file)
> > > + /*
> > > + * The desc->file may not be the same as vma->vm_file, but if the
> > > + * f_op->mmap_prepare() handler is setting this parameter to something
> > > + * different, it indicates that it wishes the VMA to have its file
> > > + * assigned to this.
> > > + */
> > > + if (orig_file != desc->file && vma->vm_file != desc->file)
> > > vma_set_file(vma, desc->file);
> >
> > So now we have to be sure both orig_file and vma->vm_file != desc->file
> > to set it? This seems to make the function name less accurate.
>
> I'll update the comment accordingly.
>
> On this in general - In future an mmap_prepare() caller may wish to change
> the file to desc->file from vma->vm_file which currently won't work for a
> stacked file system.
>
> It's pretty niche and unlikely anybody does it, but if they do, since I am
> the one implementing all this I will adjust the descriptor to send a
> separate file parameter and adjust this code accordingly.
>
> Cheers, Lorenzo
Actually, let me also send vma->vm_file in desc and make that what gets updated,
and make struct file read-only.
That way we solve this problem a lot more neatly. No users are currently setting
desc->file so it's safe to do right now.
