Skip to contentSkip to conversation

mm: don't access vm_flags as 'int'

PATCH In review KOSAKI Motohiro 5 participants 7 messages May 26, 2011 – May 27, 2011

Subsystems (5)
Memory management MM core Proc filesystem Memory mapping +1 more subsystems
lore
Share & tools
lore

KOSAKI Motohiro

author
#0 lore raw

PATCH 1/1 mm: don't access vm_flags as 'int'

The type of vma->vm_flags is 'unsigned long'. Neither 'int' nor 'unsigned int'. This patch fixes such misuse.

Signed-off-by KOSAKI Motohiro <[email protected]>
---
 fs/hugetlbfs/inode.c           |    3 ++-
 fs/proc/task_mmu.c             |    2 +-
 include/linux/hugetlb.h        |    6 +++---
 include/linux/hugetlb_inline.h |    2 +-
 include/linux/mm.h             |    6 +++---
 ipc/shm.c                      |    2 +-
 mm/fremap.c                    |    2 +-
 mm/hugetlb.c                   |    4 ++--
 mm/memory.c                    |    2 +-
 mm/mlock.c                     |    8 ++++----
 mm/mmap.c                      |    8 ++++----
 11 files changed, 23 insertions(+), 22 deletions(-)
fs/hugetlbfs/inode.c +2 -1
index e7a0357..1eb08d0 100644
@@ -921,7 +921,8 @@ static int can_do_hugetlb_shm(void)
 	return capable(CAP_IPC_LOCK) || in_group_p(sysctl_hugetlb_shm_group);
 }
 
-struct file *hugetlb_file_setup(const char *name, size_t size, int acctflag,
+struct file *hugetlb_file_setup(const char *name, size_t size,
+				unsigned long acctflag,
 				struct user_struct **user, int creat_flags)
 {
 	int error = -ENOMEM;
fs/proc/task_mmu.c +1 -1
index 2c9db29..f0f9d29 100644
@@ -211,7 +211,7 @@ static void show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct file *file = vma->vm_file;
-	int flags = vma->vm_flags;
+	unsigned long flags = vma->vm_flags;
 	unsigned long ino = 0;
 	unsigned long long pgoff = 0;
 	unsigned long start, end;
include/linux/hugetlb.h +3 -3
index 943c76b..a0e0c6a 100644
@@ -41,7 +41,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 			unsigned long address, unsigned int flags);
 int hugetlb_reserve_pages(struct inode *inode, long from, long to,
 						struct vm_area_struct *vma,
-						int acctflags);
+						unsigned long vm_flags);
 void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed);
 int dequeue_hwpoisoned_huge_page(struct page *page);
 void copy_huge_page(struct page *dst, struct page *src);

@@ -168,7 +168,7 @@ static inline struct hugetlbfs_sb_info *HUGETLBFS_SB(struct super_block *sb)
 
 extern const struct file_operations hugetlbfs_file_operations;
 extern const struct vm_operations_struct hugetlb_vm_ops;
-struct file *hugetlb_file_setup(const char *name, size_t size, int acct,
+struct file *hugetlb_file_setup(const char *name, size_t size, unsigned long acct,
 				struct user_struct **user, int creat_flags);
 int hugetlb_get_quota(struct address_space *mapping, long delta);
 void hugetlb_put_quota(struct address_space *mapping, long delta);

@@ -192,7 +192,7 @@ static inline void set_file_hugepages(struct file *file)
 #define is_file_hugepages(file)			0
 #define set_file_hugepages(file)		BUG()
 static inline struct file *hugetlb_file_setup(const char *name, size_t size,
-		int acctflag, struct user_struct **user, int creat_flags)
+		unsigned long acctflag, struct user_struct **user, int creat_flags)
 {
 	return ERR_PTR(-ENOSYS);
 }
include/linux/hugetlb_inline.h +1 -1
index 6931489..2bb681f 100644
@@ -7,7 +7,7 @@
 
 static inline int is_vm_hugetlb_page(struct vm_area_struct *vma)
 {
-	return vma->vm_flags & VM_HUGETLB;
+	return !!(vma->vm_flags & VM_HUGETLB);
 }
 
 #else
include/linux/mm.h +3 -3
index 8eb969e..6edc831 100644
@@ -165,12 +165,12 @@ extern pgprot_t protection_map[16];
  */
 static inline int is_linear_pfn_mapping(struct vm_area_struct *vma)
 {
-	return (vma->vm_flags & VM_PFN_AT_MMAP);
+	return !!(vma->vm_flags & VM_PFN_AT_MMAP);
 }
 
 static inline int is_pfn_mapping(struct vm_area_struct *vma)
 {
-	return (vma->vm_flags & VM_PFNMAP);
+	return !!(vma->vm_flags & VM_PFNMAP);
 }
 
 /*

@@ -1432,7 +1432,7 @@ extern unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
 	unsigned long flag, unsigned long pgoff);
 extern unsigned long mmap_region(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long flags,
-	unsigned int vm_flags, unsigned long pgoff);
+	unsigned long vm_flags, unsigned long pgoff);
 
 static inline unsigned long do_mmap(struct file *file, unsigned long addr,
 	unsigned long len, unsigned long prot,
ipc/shm.c +1 -1
index 729acb7..f924655 100644
@@ -347,7 +347,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	struct file * file;
 	char name[13];
 	int id;
-	int acctflag = 0;
+	unsigned long acctflag = 0;
 
 	if (size < SHMMIN || size > ns->shm_ctlmax)
 		return -EINVAL;
mm/fremap.c +1 -1
index 7f41230..59e677f 100644
@@ -224,7 +224,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
 		/*
 		 * drop PG_Mlocked flag for over-mapped range
 		 */
-		unsigned int saved_flags = vma->vm_flags;
+		unsigned long saved_flags = vma->vm_flags;
 		munlock_vma_pages_range(vma, start, start + size);
 		vma->vm_flags = saved_flags;
 	}
mm/hugetlb.c +2 -2
index 5fd68b9..450b59c 100644
@@ -2833,7 +2833,7 @@ void hugetlb_change_protection(struct vm_area_struct *vma,
 int hugetlb_reserve_pages(struct inode *inode,
 					long from, long to,
 					struct vm_area_struct *vma,
-					int acctflag)
+					unsigned long vm_flags)
 {
 	long ret, chg;
 	struct hstate *h = hstate_inode(inode);

@@ -2843,7 +2843,7 @@ int hugetlb_reserve_pages(struct inode *inode,
 	 * attempt will be made for VM_NORESERVE to allocate a page
 	 * and filesystem quota without using reserves
 	 */
-	if (acctflag & VM_NORESERVE)
+	if (vm_flags & VM_NORESERVE)
 		return 0;
 
 	/*
mm/memory.c +1 -1
index b73f677..69cfed5 100644
@@ -730,7 +730,7 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 	add_taint(TAINT_BAD_PAGE);
 }
 
-static inline int is_cow_mapping(unsigned int flags)
+static inline int is_cow_mapping(unsigned long flags)
 {
 	return (flags & (VM_SHARED | VM_MAYWRITE)) == VM_MAYWRITE;
 }
mm/mlock.c +4 -4
index 516b2c2..c92b16c 100644
@@ -307,13 +307,13 @@ void munlock_vma_pages_range(struct vm_area_struct *vma,
  * For vmas that pass the filters, merge/split as appropriate.
  */
 static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
-	unsigned long start, unsigned long end, unsigned int newflags)
+	unsigned long start, unsigned long end, unsigned long newflags)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	pgoff_t pgoff;
 	int nr_pages;
 	int ret = 0;
-	int lock = newflags & VM_LOCKED;
+	int lock = !!(newflags & VM_LOCKED);
 
 	if (newflags == vma->vm_flags || (vma->vm_flags & VM_SPECIAL) ||
 	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm))

@@ -385,7 +385,7 @@ static int do_mlock(unsigned long start, size_t len, int on)
 		prev = vma;
 
 	for (nstart = start ; ; ) {
-		unsigned int newflags;
+		unsigned long newflags;
 
 		/* Here we know that  vma->vm_start <= nstart < vma->vm_end. */
 

@@ -524,7 +524,7 @@ static int do_mlockall(int flags)
 		goto out;
 
 	for (vma = current->mm->mmap; vma ; vma = prev->vm_next) {
-		unsigned int newflags;
+		unsigned long newflags;
 
 		newflags = vma->vm_flags | VM_LOCKED;
 		if (!(flags & MCL_CURRENT))
mm/mmap.c +4 -4
index ac2631b..6ffb885 100644
@@ -960,7 +960,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
 {
 	struct mm_struct * mm = current->mm;
 	struct inode *inode;
-	unsigned int vm_flags;
+	unsigned long vm_flags;
 	int error;
 	unsigned long reqprot = prot;
 

@@ -1165,7 +1165,7 @@ SYSCALL_DEFINE1(old_mmap, struct mmap_arg_struct __user *, arg)
  */
 int vma_wants_writenotify(struct vm_area_struct *vma)
 {
-	unsigned int vm_flags = vma->vm_flags;
+	unsigned long vm_flags = vma->vm_flags;
 
 	/* If it was private or non-writable, the write bit is already clear */
 	if ((vm_flags & (VM_WRITE|VM_SHARED)) != ((VM_WRITE|VM_SHARED)))

@@ -1193,7 +1193,7 @@ int vma_wants_writenotify(struct vm_area_struct *vma)
  * We account for memory if it's a private writeable mapping,
  * not hugepages and VM_NORESERVE wasn't set.
  */
-static inline int accountable_mapping(struct file *file, unsigned int vm_flags)
+static inline int accountable_mapping(struct file *file, unsigned long vm_flags)
 {
 	/*
 	 * hugetlb has its own accounting separate from the core VM

@@ -1207,7 +1207,7 @@ static inline int accountable_mapping(struct file *file, unsigned int vm_flags)
 
 unsigned long mmap_region(struct file *file, unsigned long addr,
 			  unsigned long len, unsigned long flags,
-			  unsigned int vm_flags, unsigned long pgoff)
+			  unsigned long vm_flags, unsigned long pgoff)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma, *prev;
Signature · 9 lines
1.7.3.1


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw

2011/5/26 KOSAKI Motohiro <[email protected]>:

Quoted 2 lines

The type of vma->vm_flags is 'unsigned long'. Neither 'int' nor 'unsigned int'. This patch fixes such misuse.

I applied this, except I also just made the executive decision to replace things with "vm_flags_t" after all.

Which leaves a lot of "unsigned long" users that aren't converted, but right now it doesn't matter, and it can be converted piecemeal as people notice users..

Linus

#2 lore raw
Quoted 6 lines · Linus Torvalds

2011/5/26 KOSAKI Motohiro <[email protected]>:

Quoted 2 lines

The type of vma->vm_flags is 'unsigned long'. Neither 'int' nor 'unsigned int'. This patch fixes such misuse.

I applied this, except I also just made the executive decision to replace things with "vm_flags_t" after all.

Woo-hoo!

Why it is marked __nocast and not __bitwise__ like gfp_t?

#4 lore raw
In reply to Linus Torvalds
Quoted 11 lines · Linus Torvalds

2011/5/26 KOSAKI Motohiro <[email protected]>:

Quoted 2 lines

The type of vma->vm_flags is 'unsigned long'. Neither 'int' nor 'unsigned int'. This patch fixes such misuse.

I applied this, except I also just made the executive decision to replace things with "vm_flags_t" after all.

Which leaves a lot of "unsigned long" users that aren't converted, but right now it doesn't matter, and it can be converted piecemeal as people notice users..

This breaks kernel builds with CONFIG_HUGETLBFS=n. :-(

In file included from fs/proc/meminfo.c:2:0:
include/linux/hugetlb.h:195:3: error: expected declaration specifiers or ‘...’ before ‘vm_flags_t’
CC drivers/ata/libata-pmp.o
make[2]: *** [fs/proc/meminfo.o] Fehler 1
make[1]: *** [fs/proc] Fehler 2
make: *** [fs] Fehler 2
make: *** Warte auf noch nicht beendete Prozesse...

Signature · 9 lines
Thanks,
//richard

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#3 lore raw
In reply to Alexey Dobriyan
Quoted 4 lines · Alexey Dobriyan

Woo-hoo!

Why it is marked __nocast and not __bitwise__ like gfp_t?

Because that's what one of the other patches in Andrew's series had, so I just emulated that.

Also, I don't think we can currently mark it __bitwise without causing a sh*tload of sparse warnings. __nocast is much weaker than bitwise (it only warns about implicit casts to different sizes). __bitwise implies a lot more type-checking, and actually makes the result a very specific type.

I'm not sure it is worth the __bitwise pain. If we go down the __bitwise path, we'd need to mark all the VM_XYZZY constants with the type, and we'd need to do *all* the conversions in one go. I am definitely not ready to do that at this stage, but I was willing to take the much weaker __nocast.

Linus

#5 lore raw
In reply to richard -rw- weinberger
Quoted 2 lines · richard -rw- weinberger

This breaks kernel builds with CONFIG_HUGETLBFS=n. :-(

Grr. I did the "allyesconfig" build to find any problems, but that obviously also sets HUGETLBFS.

But "allnoconfig" does find this.

Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw
Quoted 9 lines · Linus Torvalds
Quoted 2 lines · richard -rw- weinberger

This breaks kernel builds with CONFIG_HUGETLBFS=n. :-(

Grr. I did the "allyesconfig" build to find any problems, but that obviously also sets HUGETLBFS.

But "allnoconfig" does find this.

Yeah, my experience is that [all[yes|no|mod]|def]config covers about 95% of the build bugs and the remaining 5% are rarely showstopper problems (they generally don't show up in configs that people use).

Given how fast allnoconfig and defconfig builds i've got allnoconfig and defconfig scripted to always execute together with allyesconfig.

So if you go the trouble to build allyesconfig manually, it costs almost no time to tack on the allnoconfig and defconfig as well (and stick this all into a script), while it increases the efficiency of the build test remarkably.

Thanks,

Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>