Skip to contentSkip to conversation

linux: mm_types: allow to modified the vm_flags in vm_area_struct

RFC PATCH In review Vincenzo Palazzo 4 participants 6 messages Mar 12, 2023 – Mar 13, 2023 MM core

lore
Share & tools
lore

Vincenzo Palazzo

author
#0 lore raw

RFC PATCH 1/1 linux: mm_types: allow to modified the vm_flags in vm_area_struct

With 6.3-rc1 is not possible to build the following drivers

- nvidia, that the compilation returns the following errors

/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c: In function ‘nvidia_mmap_numa’:
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:455:19: error: assignment of read-only member ‘vm_flags’

  455 |     vma->vm_flags |= VM_MIXEDMAP;
      |                   ^~
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c: In function ‘nvidia_mmap_helper’:
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:599:23: error: assignment of read-only member ‘vm_flags’
  599 |         vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND;
      |                       ^~
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:666:23: error: assignment of read-only member ‘vm_flags’
  666 |         vma->vm_flags |= (VM_IO | VM_LOCKED | VM_RESERVED);
      |                       ^~
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:667:23: error: assignment of read-only member ‘vm_flags’
  667 |         vma->vm_flags |= (VM_DONTEXPAND | VM_DONTDUMP);
      |                       ^~

/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:673:23: error: assignment of read-only member ‘vm_flags’
673 | vma->vm_flags &= ~VM_WRITE;
| ^~
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:674:23: error: assignment of read-only member ‘vm_flags’
674 | vma->vm_flags &= ~VM_MAYWRITE;
| ^~
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c: In function ‘nv_encode_caching’:
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:353:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
353 | if (NV_ALLOW_CACHING(memory_type))
| ^
/var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.c:356:9: note: here

  356 |         default:
      |         ^~~~~~~

make[2]: *** [scripts/Makefile.build:252: /var/lib/dkms/nvidia/525.89.02/build/nvidia/nv-mmap.o] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:2028: /var/lib/dkms/nvidia/525.89.02/build] Error 2
make[1]: Leaving directory '/home/vincenzopalazzo/gittea/linux'
make: *** [Makefile:82: modules] Error 2
------

- vboxhost/7.0.6_OSE where the build fails with the following errors DKMS make.log for vboxhost-7.0.6_OSE for kernel 6.3.0-rc1-00231-g511e7733223b (x86_64) Sat Mar 11 04:21:34 PM UTC 2023

  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/linux/VBoxNetFlt-linux.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetadp/linux/VBoxNetAdp-linux.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/VBoxNetFlt.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/linux/SUPDrv-linux.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/SUPDrv.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetadp/VBoxNetAdp.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/SUPDrvGip.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/SUPR0IdcClient.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/SUPDrvSem.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/SUPR0IdcClientComponent.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/linux/SUPR0IdcClient-linux.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/SUPDrvTracer.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/SUPLibAll.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/common/string/strformatrt.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/combined-agnostic1.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/combined-agnostic2.o
  CC [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/combined-os-specific.o
  LD [M]  /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetadp/vboxnetadp.o
In file included from /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/combined-os-specific.c:43:
/var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/r0drv/linux/memobj-r0drv-linux.c: In function ‘rtR0MemObjNativeLockUser’:
/var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/r0drv/linux/memobj-r0drv-linux.c:1404:39: error: assignment of read-only member ‘vm_flags’
 1404 |                 papVMAs[rc]->vm_flags |= VM_DONTCOPY | VM_LOCKED;
      |                                       ^~
/var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/r0drv/linux/memobj-r0drv-linux.c: In function ‘rtR0MemObjNativeMapUser’:
/var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/r0drv/linux/memobj-r0drv-linux.c:1876:35: error: assignment of read-only member ‘vm_flags’
 1876 |                     vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
      |                                   ^~

make[2]: *** [scripts/Makefile.build:252: /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv/combined-os-specific.o] Error 1 make[2]: *** Waiting for unfinished jobs....
LD [M] /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxnetflt/vboxnetflt.o
make[1]: *** [scripts/Makefile.build:494: /var/lib/dkms/vboxhost/7.0.6_OSE/build/vboxdrv] Error 2
make: *** [Makefile:2028: /var/lib/dkms/vboxhost/7.0.6_OSE/build] Error 2
-----

The commands that I use to build these drivers are:

- sudo dkms install --no-depmod nvidia/525.89.02 -k 6.3.0-rc1-00231-g511e7733223b
- sudo dkms install --no-depmod vboxhost/7.0.6_OSE -k 6.3.0-rc1-00231-g511e7733223b

The following commit reverts the const constraint that was added in [1], and also I tested this patch on my system and all started again as 6.2.2.

[1] https://lore.kernel.org/all/[email protected]/T/#u

Signed-off-by Vincenzo Palazzo <[email protected]>
---
 include/linux/mm_types.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/mm_types.h +1 -1
index 0722859c3647..a354eb60c7e8 100644
@@ -497,7 +497,7 @@ struct vm_area_struct {
 	 * To modify use vm_flags_{init|reset|set|clear|mod} functions.
 	 */
 	union {
-		const vm_flags_t vm_flags;
+		vm_flags_t vm_flags;
 		vm_flags_t __private __vm_flags;
 	};
Signature · 6 lines
2.39.2

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
maintainer
#1 lore raw
Quoted 1 line · On Sun, 12 Mar 2023 23:42:50 +0100 Vincenzo Palazzo…

With 6.3-rc1 is not possible to build the following drivers

Well, let's cc Suren who actually wrote bc292ab00f6c ("mm: introduce vma->vm_flags wrapper functions").

Quoted 9 lines

- nvidia, that the compilation returns the following errors

- vboxhost/7.0.6_OSE where the build fails with the following errors

...

include/linux/mm_types.h struct vm_area_struct { · line 502

Quoted 7 lines
include/linux/mm_types.h +1 -1
@@ -497,7 +497,7 @@ struct vm_area_struct {
 	 * To modify use vm_flags_{init|reset|set|clear|mod} functions.
 	 */
 	union {
-		const vm_flags_t vm_flags;
+		vm_flags_t vm_flags;
 		vm_flags_t __private __vm_flags;
 	};

These out-of-tree drivers should be converted to use the wrappers which bc292ab00f6c added. Their maintainers have a couple of months to do this before bc292ab00f6c is released in 6.3.

It's unfortunate. I guess it would be better if we were to find a way to permit these drivers to compile OK but to generate warnings, as an interim thing for six months or so. Perhaps there's some clever way.

author
#2 lore raw
Quoted 13 lines · Andrew Morton
Quoted 1 line · On Sun, 12 Mar 2023 23:42:50 +0100 Vincenzo Palazzo…

With 6.3-rc1 is not possible to build the following drivers

Well, let's cc Suren who actually wrote bc292ab00f6c ("mm: introduce vma->vm_flags wrapper functions").

Quoted 6 lines

- nvidia, that the compilation returns the following errors

- vboxhost/7.0.6_OSE where the build fails with the following errors

...

Quoted 14 lines

include/linux/mm_types.h struct vm_area_struct { · line 502

Quoted 10 lines
include/linux/mm_types.h +1 -1
@@ -497,7 +497,7 @@ struct vm_area_struct {
 	 * To modify use vm_flags_{init|reset|set|clear|mod} functions.
 	 */
 	union {
-		const vm_flags_t vm_flags;
+		vm_flags_t vm_flags;
 		vm_flags_t __private __vm_flags;
 	};

These out-of-tree drivers should be converted to use the wrappers which bc292ab00f6c added. Their maintainers have a couple of months to do this before bc292ab00f6c is released in 6.3.

Yeah I thought about that! nvidia update as soon as the 6.3 is out, not sure about vboxhost. But let the driver update to this new feature make sense.

Quoted 4 lines

It's unfortunate. I guess it would be better if we were to find a way to permit these drivers to compile OK but to generate warnings, as an interim thing for six months or so. Perhaps there's some clever way.

Yeah it is! Regarding giving the possiblity to compile ok (and preserving the previous behavior) look like an interesting thought for kernel API, but am I think that it this is difficult to do?

The real things that I'm missing with the new wrapper API is the meaning of the new vm_flags_t vm_flags, before the field was only one and it was getting modified, but now the real value is the private one __vm_flags, so what is the meaning of the new vm_flags?

I feel that this question is stupid but I can not avoid do it, otherwise my mind think about it till I will not find an answer :)

P.S: regaring the warning at compile time the __deprecated looks a good fit to generate error message, but to me in this particular case do not.

Thanks,

Vincent.

#3 lore raw
Quoted 2 lines · Vincenzo Palazzo

P.S: regaring the warning at compile time the __deprecated looks a good fit to generate error message, but to me in this particular case do not.

__deprecated does not work in the kernel tree, sorry. We have tried that in the past and it just gets ignored. Instead, we fix up all in-kernel users and continue on.

thanks,

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

reviewer
#4 lore raw
In reply to Vincenzo Palazzo
Quoted 42 lines · Vincenzo Palazzo
Quoted 13 lines · Andrew Morton
Quoted 1 line · On Sun, 12 Mar 2023 23:42:50 +0100 Vincenzo Palazzo…

With 6.3-rc1 is not possible to build the following drivers

Well, let's cc Suren who actually wrote bc292ab00f6c ("mm: introduce vma->vm_flags wrapper functions").

Quoted 6 lines

- nvidia, that the compilation returns the following errors

- vboxhost/7.0.6_OSE where the build fails with the following errors

...

Quoted 14 lines

include/linux/mm_types.h struct vm_area_struct { · line 502

Quoted 10 lines
include/linux/mm_types.h +1 -1
@@ -497,7 +497,7 @@ struct vm_area_struct {
     * To modify use vm_flags_{init|reset|set|clear|mod} functions.
     */
    union {
-           const vm_flags_t vm_flags;
+           vm_flags_t vm_flags;
            vm_flags_t __private __vm_flags;
    };

These out-of-tree drivers should be converted to use the wrappers which bc292ab00f6c added. Their maintainers have a couple of months to do this before bc292ab00f6c is released in 6.3.

Yeah I thought about that! nvidia update as soon as the 6.3 is out, not sure about vboxhost. But let the driver update to this new feature make sense.

Quoted 4 lines

It's unfortunate. I guess it would be better if we were to find a way to permit these drivers to compile OK but to generate warnings, as an interim thing for six months or so. Perhaps there's some clever way.

Yeah it is! Regarding giving the possiblity to compile ok (and preserving the previous behavior) look like an interesting thought for kernel API, but am I think that it this is difficult to do?

Quoted 5 lines

The real things that I'm missing with the new wrapper API is the meaning of the new vm_flags_t vm_flags, before the field was only one and it was getting modified, but now the real value is the private one __vm_flags, so what is the meaning of the new vm_flags?

Sorry for the delay. I'm on vacation until Wednesday and have limited network access.
vm_flags is used as a read-only property so that readers of the flags don't have to change, however flag modifiers should be using new wrapper functions. Fixing the errors in this report should be trivial. For example:
vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND;
should be replaced with
vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND);
and
vma->vm_flags &= ~VM_MAYWRITE;
with
vm_flags_clear(vma, VM_MAYWRITE);

Thanks,
Suren.

Quoted 11 lines

I feel that this question is stupid but I can not avoid do it, otherwise my mind think about it till I will not find an answer :)

P.S: regaring the warning at compile time the __deprecated looks a good fit to generate error message, but to me in this particular case do not.

Thanks,

Vincent.

_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

#5 lore raw
In reply to Vincenzo Palazzo
Quoted 3 lines · Vincenzo Palazzo

With 6.3-rc1 is not possible to build the following drivers

- nvidia, that the compilation returns the following errors

<snip>

Quoted 1 line

- vboxhost/7.0.6_OSE where the build fails with the following errors

Out-of-tree code needs to be fixed up all the time, this is their responsibility, not the kernel's responsibility. Please submit changes to their codebases if you rely on these modules.

thanks,

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees