[10/13] mm: Wire up MAP_SYNC

Message ID 20170817160815.30466-11-jack@suse.cz
State New
Headers show

Commit Message

Jan Kara Aug. 17, 2017, 4:08 p.m.
Pretty crude for now...

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/file.c                  | 2 ++
 include/linux/mm.h              | 1 +
 include/linux/mman.h            | 3 ++-
 include/uapi/asm-generic/mman.h | 1 +
 mm/mmap.c                       | 5 +++++
 5 files changed, 11 insertions(+), 1 deletion(-)

Comments

Ross Zwisler Aug. 21, 2017, 9:37 p.m. | #1
On Thu, Aug 17, 2017 at 06:08:12PM +0200, Jan Kara wrote:
> Pretty crude for now...
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/file.c                  | 2 ++
>  include/linux/mm.h              | 1 +
>  include/linux/mman.h            | 3 ++-
>  include/uapi/asm-generic/mman.h | 1 +
>  mm/mmap.c                       | 5 +++++
>  5 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index f84bb29e941e..850037e140d7 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -340,6 +340,8 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
>  		vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
>  	} else {
>  		vma->vm_ops = &ext4_file_vm_ops;
> +		if (vma->vm_flags & VM_SYNC)
> +			return -EOPNOTSUPP;
>  	}
>  	return 0;
>  }
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index fa036093e76c..d0fb385414a4 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -188,6 +188,7 @@ extern unsigned int kobjsize(const void *objp);
>  #define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
>  #define VM_NORESERVE	0x00200000	/* should the VM suppress accounting */
>  #define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
> +#define VM_SYNC		0x00800000	/* Synchronous page faults */
>  #define VM_ARCH_1	0x01000000	/* Architecture-specific flag */
>  #define VM_ARCH_2	0x02000000
>  #define VM_DONTDUMP	0x04000000	/* Do not include in the core dump */
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index c8367041fafd..c38279b651e5 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -86,7 +86,8 @@ calc_vm_flag_bits(unsigned long flags)
>  {
>  	return _calc_vm_trans(flags, MAP_GROWSDOWN,  VM_GROWSDOWN ) |
>  	       _calc_vm_trans(flags, MAP_DENYWRITE,  VM_DENYWRITE ) |
> -	       _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    );
> +	       _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    ) |
> +	       _calc_vm_trans(flags, MAP_SYNC,	     VM_SYNC      );
>  }
>  
>  unsigned long vm_commit_limit(void);
> diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
> index 7162cd4cca73..00e55627d2df 100644
> --- a/include/uapi/asm-generic/mman.h
> +++ b/include/uapi/asm-generic/mman.h
> @@ -12,6 +12,7 @@
>  #define MAP_NONBLOCK	0x10000		/* do not block on IO */
>  #define MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */
>  #define MAP_HUGETLB	0x40000		/* create a huge page mapping */
> +#define MAP_SYNC	0x80000		/* perform synchronous page faults for the mapping */
>  
>  /* Bits [26:31] are reserved, see mman-common.h for MAP_HUGETLB usage */
>  
> diff --git a/mm/mmap.c b/mm/mmap.c
> index f19efcf75418..18453c04b09f 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1423,12 +1423,17 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
>  				return -ENODEV;
>  			if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP))
>  				return -EINVAL;
> +			if (!(vm_flags & VM_SHARED) && (vm_flags & VM_SYNC))
> +				return -EINVAL;

I know this will be reworked with Dan's new mmap() interface, but I was
curious what the !(vm_flags & VM_SHARED) check here was for.  We're in a
MAP_PRIVATE case, so is it ever possible for VM_SHARED to be set in vm_flags?
I tried to make this happen with some various test scenarios, but wasn't able.

>  			break;
>  
>  		default:
>  			return -EINVAL;
>  		}
>  	} else {
> +		if (vm_flags & VM_SYNC)
> +			return -EINVAL;
> +
>  		switch (flags & MAP_TYPE) {
>  		case MAP_SHARED:
>  			if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP))
> -- 
> 2.12.3
>
Jan Kara Aug. 22, 2017, 9:36 a.m. | #2
On Mon 21-08-17 15:37:04, Ross Zwisler wrote:
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index f19efcf75418..18453c04b09f 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1423,12 +1423,17 @@ unsigned long do_mmap(struct file *file, unsigned long addr,
> >  				return -ENODEV;
> >  			if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP))
> >  				return -EINVAL;
> > +			if (!(vm_flags & VM_SHARED) && (vm_flags & VM_SYNC))
> > +				return -EINVAL;
> 
> I know this will be reworked with Dan's new mmap() interface, but I was
> curious what the !(vm_flags & VM_SHARED) check here was for.  We're in a
> MAP_PRIVATE case, so is it ever possible for VM_SHARED to be set in vm_flags?
> I tried to make this happen with some various test scenarios, but wasn't able.

I was also caught by this :). Check how MAP_SHARED case above falls through
to the MAP_PRIVATE case...

								Honza
Christoph Hellwig Aug. 23, 2017, 6:43 p.m. | #3
On Thu, Aug 17, 2017 at 06:08:12PM +0200, Jan Kara wrote:
> Pretty crude for now...
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/file.c                  | 2 ++
>  include/linux/mm.h              | 1 +
>  include/linux/mman.h            | 3 ++-
>  include/uapi/asm-generic/mman.h | 1 +
>  mm/mmap.c                       | 5 +++++
>  5 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index f84bb29e941e..850037e140d7 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -340,6 +340,8 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
>  		vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
>  	} else {
>  		vma->vm_ops = &ext4_file_vm_ops;
> +		if (vma->vm_flags & VM_SYNC)
> +			return -EOPNOTSUPP;
>  	}

So each mmap instance would need to reject the flag explicitly?

Or do I misunderstand this VM_SYNC flag?
Jan Kara Aug. 24, 2017, 7:16 a.m. | #4
On Wed 23-08-17 11:43:49, Christoph Hellwig wrote:
> On Thu, Aug 17, 2017 at 06:08:12PM +0200, Jan Kara wrote:
> > Pretty crude for now...
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/ext4/file.c                  | 2 ++
> >  include/linux/mm.h              | 1 +
> >  include/linux/mman.h            | 3 ++-
> >  include/uapi/asm-generic/mman.h | 1 +
> >  mm/mmap.c                       | 5 +++++
> >  5 files changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> > index f84bb29e941e..850037e140d7 100644
> > --- a/fs/ext4/file.c
> > +++ b/fs/ext4/file.c
> > @@ -340,6 +340,8 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
> >  		vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
> >  	} else {
> >  		vma->vm_ops = &ext4_file_vm_ops;
> > +		if (vma->vm_flags & VM_SYNC)
> > +			return -EOPNOTSUPP;
> >  	}
> 
> So each mmap instance would need to reject the flag explicitly?
> 
> Or do I misunderstand this VM_SYNC flag?

Yes, if this should be cleaned up, then each mmap instance not supporting
it would need to reject it. However Dan has in his version of mmap()
syscall a mask of supported flags so when I switch to that, it would be
just opt-in. Or I could just reject VM_SYNC for any !IS_DAX inode so then
only ext2 & xfs would need to reject it... But the biggest problem with
this patch is that we need to settle on a safe way of adding new mmap flag.

								Honza

Patch

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index f84bb29e941e..850037e140d7 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -340,6 +340,8 @@  static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
 		vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;
 	} else {
 		vma->vm_ops = &ext4_file_vm_ops;
+		if (vma->vm_flags & VM_SYNC)
+			return -EOPNOTSUPP;
 	}
 	return 0;
 }
diff --git a/include/linux/mm.h b/include/linux/mm.h
index fa036093e76c..d0fb385414a4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -188,6 +188,7 @@  extern unsigned int kobjsize(const void *objp);
 #define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
 #define VM_NORESERVE	0x00200000	/* should the VM suppress accounting */
 #define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
+#define VM_SYNC		0x00800000	/* Synchronous page faults */
 #define VM_ARCH_1	0x01000000	/* Architecture-specific flag */
 #define VM_ARCH_2	0x02000000
 #define VM_DONTDUMP	0x04000000	/* Do not include in the core dump */
diff --git a/include/linux/mman.h b/include/linux/mman.h
index c8367041fafd..c38279b651e5 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -86,7 +86,8 @@  calc_vm_flag_bits(unsigned long flags)
 {
 	return _calc_vm_trans(flags, MAP_GROWSDOWN,  VM_GROWSDOWN ) |
 	       _calc_vm_trans(flags, MAP_DENYWRITE,  VM_DENYWRITE ) |
-	       _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    );
+	       _calc_vm_trans(flags, MAP_LOCKED,     VM_LOCKED    ) |
+	       _calc_vm_trans(flags, MAP_SYNC,	     VM_SYNC      );
 }
 
 unsigned long vm_commit_limit(void);
diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
index 7162cd4cca73..00e55627d2df 100644
--- a/include/uapi/asm-generic/mman.h
+++ b/include/uapi/asm-generic/mman.h
@@ -12,6 +12,7 @@ 
 #define MAP_NONBLOCK	0x10000		/* do not block on IO */
 #define MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */
 #define MAP_HUGETLB	0x40000		/* create a huge page mapping */
+#define MAP_SYNC	0x80000		/* perform synchronous page faults for the mapping */
 
 /* Bits [26:31] are reserved, see mman-common.h for MAP_HUGETLB usage */
 
diff --git a/mm/mmap.c b/mm/mmap.c
index f19efcf75418..18453c04b09f 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1423,12 +1423,17 @@  unsigned long do_mmap(struct file *file, unsigned long addr,
 				return -ENODEV;
 			if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP))
 				return -EINVAL;
+			if (!(vm_flags & VM_SHARED) && (vm_flags & VM_SYNC))
+				return -EINVAL;
 			break;
 
 		default:
 			return -EINVAL;
 		}
 	} else {
+		if (vm_flags & VM_SYNC)
+			return -EINVAL;
+
 		switch (flags & MAP_TYPE) {
 		case MAP_SHARED:
 			if (vm_flags & (VM_GROWSDOWN|VM_GROWSUP))