vma_start_write() is used in many places and will grow in size very soon.
It is not used in performance critical paths and uninlining it should
limit the future code size growth.
No functional changes.
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
include/linux/mm.h | 12 +++---------
mm/memory.c | 14 ++++++++++++++
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ab27de9729d8..ea4c4228b125 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -787,6 +787,8 @@ static bool __is_vma_write_locked(struct vm_area_struct *vma, unsigned int *mm_l
return (vma->vm_lock_seq == *mm_lock_seq);
}
+void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq);
+
/*
* Begin writing to a VMA.
* Exclude concurrent readers under the per-VMA lock until the currently
@@ -799,15 +801,7 @@ static inline void vma_start_write(struct vm_area_struct *vma)
if (__is_vma_write_locked(vma, &mm_lock_seq))
return;
- down_write(&vma->vm_lock.lock);
- /*
- * We should use WRITE_ONCE() here because we can have concurrent reads
- * from the early lockless pessimistic check in vma_start_read().
- * We don't really care about the correctness of that early check, but
- * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
- */
- WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
- up_write(&vma->vm_lock.lock);
+ __vma_start_write(vma, mm_lock_seq);
}
static inline void vma_assert_write_locked(struct vm_area_struct *vma)
diff --git a/mm/memory.c b/mm/memory.c
index d0dee2282325..236fdecd44d6 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6328,6 +6328,20 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
#endif
#ifdef CONFIG_PER_VMA_LOCK
+void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq)
+{
+ down_write(&vma->vm_lock.lock);
+ /*
+ * We should use WRITE_ONCE() here because we can have concurrent reads
+ * from the early lockless pessimistic check in vma_start_read().
+ * We don't really care about the correctness of that early check, but
+ * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
+ */
+ WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
+ up_write(&vma->vm_lock.lock);
+}
+EXPORT_SYMBOL_GPL(__vma_start_write);
+
/*
* Lookup and lock a VMA under RCU protection. Returned VMA is guaranteed to be
* stable and not isolated. If the VMA is not found or is being modified the
--
2.47.1.613.gc27f4b7a9f-googOn 12/26/24 18:07, Suren Baghdasaryan wrote:
> vma_start_write() is used in many places and will grow in size very soon.
> It is not used in performance critical paths and uninlining it should
> limit the future code size growth.
> No functional changes.
>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6328,6 +6328,20 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> #endif
>
> #ifdef CONFIG_PER_VMA_LOCK
> +void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq)
> +{
> + down_write(&vma->vm_lock.lock);
> + /*
> + * We should use WRITE_ONCE() here because we can have concurrent reads
> + * from the early lockless pessimistic check in vma_start_read().
> + * We don't really care about the correctness of that early check, but
> + * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
> + */
> + WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
> + up_write(&vma->vm_lock.lock);
> +}
> +EXPORT_SYMBOL_GPL(__vma_start_write);
Do any modules need it? If not we shouldn't export.
> /*
> * Lookup and lock a VMA under RCU protection. Returned VMA is guaranteed to be
> * stable and not isolated. If the VMA is not found or is being modified the
On 1/8/25 01:35, Vlastimil Babka wrote:
> On 12/26/24 18:07, Suren Baghdasaryan wrote:
>> vma_start_write() is used in many places and will grow in size very soon.
>> It is not used in performance critical paths and uninlining it should
>> limit the future code size growth.
>> No functional changes.
>>
>> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -6328,6 +6328,20 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
>> #endif
>>
>> #ifdef CONFIG_PER_VMA_LOCK
>> +void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq)
>> +{
>> + down_write(&vma->vm_lock.lock);
>> + /*
>> + * We should use WRITE_ONCE() here because we can have concurrent reads
>> + * from the early lockless pessimistic check in vma_start_read().
>> + * We don't really care about the correctness of that early check, but
>> + * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
>> + */
>> + WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
>> + up_write(&vma->vm_lock.lock);
>> +}
>> +EXPORT_SYMBOL_GPL(__vma_start_write);
>
> Do any modules need it? If not we shouldn't export.
Hi Vlastimil, Suren
The out-of-tree NVIDIA modules seem to rely on this symbol, is it possible to use EXPORT_SYMBOL() here instead of EXPORT_SYMBOL_GPL(), below is the modpost error:
MODPOST Module.symvers
WARNING: modpost: missing MODULE_DESCRIPTION() in nvidia.o
WARNING: modpost: missing MODULE_DESCRIPTION() in nvidia-uvm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in nvidia-modeset.o
WARNING: modpost: missing MODULE_DESCRIPTION() in nvidia-drm.o
ERROR: modpost: GPL-incompatible module nvidia.ko uses GPL-only symbol '__vma_start_write'
ERROR: modpost: GPL-incompatible module nvidia-drm.ko uses GPL-only symbol '__vma_start_write'
make[4]: *** [/tmp/makepkg/linux-cachyos-rc-nc/src/linux-6.15-rc1/scripts/Makefile.modpost:147: Module.symvers] Error 1
make[3]: *** [/tmp/makepkg/linux-cachyos-rc-nc/src/linux-6.15-rc1/Makefile:1964: modpost] Error 2
If it's possible I can send a patch that changes that.
>
>> /*
>> * Lookup and lock a VMA under RCU protection. Returned VMA is guaranteed to be
>> * stable and not isolated. If the VMA is not found or is being modified the
>
--
Regards,
Eric
On Tue, Apr 08, 2025 at 12:39:25PM +0800, Eric Naim wrote:
> The out-of-tree NVIDIA modules seem to rely on this symbol, is it possible to use EXPORT_SYMBOL() here instead of EXPORT_SYMBOL_GPL(), below is the modpost error:
No. They don't have any business using this.
In fact vma_start_write should not be exported at all, just the
vm_flags_{set,clear,mod} helpers.
On Mon, Apr 07, 2025 at 11:01:46PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 08, 2025 at 12:39:25PM +0800, Eric Naim wrote:
> > The out-of-tree NVIDIA modules seem to rely on this symbol, is it possible to use EXPORT_SYMBOL() here instead of EXPORT_SYMBOL_GPL(), below is the modpost error:
>
> No. They don't have any business using this.
What on _earth_ are they using this for? Is this just via the VMA flag
manipulation functions? If it's something else, it's an unintended use of this.
Anyway, generally speaking - agreed, this is absolutely a no-go Eric. In my view
we simply should not be using EXPORT_SYMBOL() for _any_ new symbols whatsoever.
Out-of-tree modules are simply a non-consideration for core mm code, this is a
GPL open source project. If I had my way we'd simply revoke _all_
EXPORT_SYMBOL()'s, not add new ones.
>
> In fact vma_start_write should not be exported at all, just the
> vm_flags_{set,clear,mod} helpers.
Yup, I'd rather we just kept vma_start_write() mm-internal, though of course
kernel/fork.c (ugh) needs it (we could probably refactor that in some way to
avoid), and literally just the PPC arch (again maybe we can find a way round
that).
Maybe one for me to look at actually... hmm.
Anyway Eric - I wonder if this is simply the nvidia OOT driver doing a
vm_flags_...() call and then having an issue because the lock is uninlined now?
I guess you are jut noticing this is breaking and don't know since - proprietary
code.
Anyway in this case, the OOT driver should just write some GPL wrapper code or
something here. Or better yet - make the driver open source :)
On 4/8/25 14:25, Lorenzo Stoakes wrote:
> On Mon, Apr 07, 2025 at 11:01:46PM -0700, Christoph Hellwig wrote:
>> On Tue, Apr 08, 2025 at 12:39:25PM +0800, Eric Naim wrote:
>>> The out-of-tree NVIDIA modules seem to rely on this symbol, is it possible to use EXPORT_SYMBOL() here instead of EXPORT_SYMBOL_GPL(), below is the modpost error:
>>
>> No. They don't have any business using this.
>
> What on _earth_ are they using this for? Is this just via the VMA flag
> manipulation functions? If it's something else, it's an unintended use of this.
>
> Anyway, generally speaking - agreed, this is absolutely a no-go Eric. In my view
> we simply should not be using EXPORT_SYMBOL() for _any_ new symbols whatsoever.
>
> Out-of-tree modules are simply a non-consideration for core mm code, this is a
> GPL open source project. If I had my way we'd simply revoke _all_
> EXPORT_SYMBOL()'s, not add new ones.
>
>>
>> In fact vma_start_write should not be exported at all, just the
>> vm_flags_{set,clear,mod} helpers.
>
> Yup, I'd rather we just kept vma_start_write() mm-internal, though of course
> kernel/fork.c (ugh) needs it (we could probably refactor that in some way to
> avoid), and literally just the PPC arch (again maybe we can find a way round
> that).
>
> Maybe one for me to look at actually... hmm.
>
> Anyway Eric - I wonder if this is simply the nvidia OOT driver doing a
> vm_flags_...() call and then having an issue because the lock is uninlined now?
>
> I guess you are jut noticing this is breaking and don't know since - proprietary
> code.
This seems to be the case, upon looking a bit deeper it looks like the driver code
is calling atleast one of vm_flags_set. I couldn't find any direct calls to {,__}vma_start_write
at first and was bit confused.
>
> Anyway in this case, the OOT driver should just write some GPL wrapper code or
> something here. Or better yet - make the driver open source :)
Yeah, as obvious as it seems it doesn't happen on their open-sourced code :)
Either way, I'm thankful for the replies. NVIDIA *should* have probably noticed
this already and it would probably fixed in the next driver or two so I'll just
let sleeping dogs lie.
--
Regards,
Eric
On Tue, Apr 8, 2025 at 12:52 AM Eric Naim <dnaim@cachyos.org> wrote:
>
> On 4/8/25 14:25, Lorenzo Stoakes wrote:
> > On Mon, Apr 07, 2025 at 11:01:46PM -0700, Christoph Hellwig wrote:
> >> On Tue, Apr 08, 2025 at 12:39:25PM +0800, Eric Naim wrote:
> >>> The out-of-tree NVIDIA modules seem to rely on this symbol, is it possible to use EXPORT_SYMBOL() here instead of EXPORT_SYMBOL_GPL(), below is the modpost error:
> >>
> >> No. They don't have any business using this.
> >
> > What on _earth_ are they using this for? Is this just via the VMA flag
> > manipulation functions? If it's something else, it's an unintended use of this.
> >
> > Anyway, generally speaking - agreed, this is absolutely a no-go Eric. In my view
> > we simply should not be using EXPORT_SYMBOL() for _any_ new symbols whatsoever.
> >
> > Out-of-tree modules are simply a non-consideration for core mm code, this is a
> > GPL open source project. If I had my way we'd simply revoke _all_
> > EXPORT_SYMBOL()'s, not add new ones.
> >
> >>
> >> In fact vma_start_write should not be exported at all, just the
> >> vm_flags_{set,clear,mod} helpers.
> >
> > Yup, I'd rather we just kept vma_start_write() mm-internal, though of course
> > kernel/fork.c (ugh) needs it (we could probably refactor that in some way to
> > avoid), and literally just the PPC arch (again maybe we can find a way round
> > that).
> >
> > Maybe one for me to look at actually... hmm.
> >
> > Anyway Eric - I wonder if this is simply the nvidia OOT driver doing a
> > vm_flags_...() call and then having an issue because the lock is uninlined now?
> >
> > I guess you are jut noticing this is breaking and don't know since - proprietary
> > code.
>
>
> This seems to be the case, upon looking a bit deeper it looks like the driver code
> is calling atleast one of vm_flags_set. I couldn't find any direct calls to {,__}vma_start_write
> at first and was bit confused.
I agree that EXPORT_SYMBOL_GPL() should be enough as far as exporting goes.
>
> >
> > Anyway in this case, the OOT driver should just write some GPL wrapper code or
> > something here. Or better yet - make the driver open source :)
>
> Yeah, as obvious as it seems it doesn't happen on their open-sourced code :)
>
> Either way, I'm thankful for the replies. NVIDIA *should* have probably noticed
> this already and it would probably fixed in the next driver or two so I'll just
> let sleeping dogs lie.
>
>
> --
> Regards,
> Eric
On Tue, Jan 7, 2025 at 9:35 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 12/26/24 18:07, Suren Baghdasaryan wrote:
> > vma_start_write() is used in many places and will grow in size very soon.
> > It is not used in performance critical paths and uninlining it should
> > limit the future code size growth.
> > No functional changes.
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -6328,6 +6328,20 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> > #endif
> >
> > #ifdef CONFIG_PER_VMA_LOCK
> > +void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq)
> > +{
> > + down_write(&vma->vm_lock.lock);
> > + /*
> > + * We should use WRITE_ONCE() here because we can have concurrent reads
> > + * from the early lockless pessimistic check in vma_start_read().
> > + * We don't really care about the correctness of that early check, but
> > + * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
> > + */
> > + WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
> > + up_write(&vma->vm_lock.lock);
> > +}
> > +EXPORT_SYMBOL_GPL(__vma_start_write);
>
> Do any modules need it? If not we shouldn't export.
I'm pretty sure I added it because of the allmodconfig build failure
but let me rerun it and see which module was using it.
>
> > /*
> > * Lookup and lock a VMA under RCU protection. Returned VMA is guaranteed to be
> > * stable and not isolated. If the VMA is not found or is being modified the
>
On Tue, Jan 7, 2025 at 9:45 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Tue, Jan 7, 2025 at 9:35 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 12/26/24 18:07, Suren Baghdasaryan wrote:
> > > vma_start_write() is used in many places and will grow in size very soon.
> > > It is not used in performance critical paths and uninlining it should
> > > limit the future code size growth.
> > > No functional changes.
> > >
> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> >
> > Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> >
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -6328,6 +6328,20 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
> > > #endif
> > >
> > > #ifdef CONFIG_PER_VMA_LOCK
> > > +void __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq)
> > > +{
> > > + down_write(&vma->vm_lock.lock);
> > > + /*
> > > + * We should use WRITE_ONCE() here because we can have concurrent reads
> > > + * from the early lockless pessimistic check in vma_start_read().
> > > + * We don't really care about the correctness of that early check, but
> > > + * we should use WRITE_ONCE() for cleanliness and to keep KCSAN happy.
> > > + */
> > > + WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
> > > + up_write(&vma->vm_lock.lock);
> > > +}
> > > +EXPORT_SYMBOL_GPL(__vma_start_write);
> >
> > Do any modules need it? If not we shouldn't export.
>
> I'm pretty sure I added it because of the allmodconfig build failure
> but let me rerun it and see which module was using it.
Yeah, when building allmodconfig without this export I get:
ERROR: modpost: "__vma_start_write" [fs/ext4/ext4.ko] undefined!
ERROR: modpost: "__vma_start_write" [fs/cramfs/cramfs.ko] undefined!
ERROR: modpost: "__vma_start_write" [fs/fuse/fuse.ko] undefined!
ERROR: modpost: "__vma_start_write" [fs/orangefs/orangefs.ko] undefined!
ERROR: modpost: "__vma_start_write" [fs/xfs/xfs.ko] undefined!
ERROR: modpost: "__vma_start_write" [fs/erofs/erofs.ko] undefined!
ERROR: modpost: "__vma_start_write" [drivers/video/fbdev/core/fb.ko] undefined!
ERROR: modpost: "__vma_start_write" [drivers/acpi/pfr_telemetry.ko] undefined!
ERROR: modpost: "__vma_start_write" [drivers/dma/idxd/idxd.ko] undefined!
ERROR: modpost: "__vma_start_write" [drivers/xen/xen-gntdev.ko] undefined!
>
> >
> > > /*
> > > * Lookup and lock a VMA under RCU protection. Returned VMA is guaranteed to be
> > > * stable and not isolated. If the VMA is not found or is being modified the
> >
© 2016 - 2026 Red Hat, Inc.