linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
@ 2022-06-10  9:25 Wang Wenhu
  2022-06-10  9:41 ` Greg Kroah-Hartman
  2022-06-10 13:41 ` [PATCH v2] " Wang Wenhu
  0 siblings, 2 replies; 5+ messages in thread
From: Wang Wenhu @ 2022-06-10  9:25 UTC (permalink / raw)
  To: Arnd Bergmann, Greg Kroah-Hartman, Wu Hao, Tom Rix,
	Moritz Fischer, Xu Yilun, Bjorn Helgaas, Andrew Morton,
	open list, open list:FPGA DFL DRIVERS, open list:PCI SUBSYSTEM,
	open list:MEMORY MANAGEMENT
  Cc: wenhu.wang, Wang Wenhu

It is recommended in the "Conditional Compilation" chapter of kernel
coding-style documentation that preprocessor conditionals should not
be used in .c files wherever possible.

As for the micro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
to eliminate it in .c files as we add a no-op function defination
in the header file if the micro is not enabled.

The main trigger for this patch is an UIO driver series and as Greg
commented we'd better not use such preprocessor contionals.
See: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/
For there is little work to do with the UIO driver, I try to push
this commit independently.

Signed-off-by: Wang Wenhu <lonehugo@hotmail.com>
---
 drivers/char/mem.c          | 2 --
 drivers/fpga/dfl-afu-main.c | 2 --
 drivers/pci/mmap.c          | 2 --
 drivers/uio/uio.c           | 2 --
 include/linux/mm.h          | 8 ++++++++
 mm/memory.c                 | 4 ----
 6 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 84ca98ed1dad..40186a441e38 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
 #endif
 
 static const struct vm_operations_struct mmap_mem_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys
-#endif
 };
 
 static int mmap_mem(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 7f621e96d3b8..833e14806c7a 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 }
 
 static const struct vm_operations_struct afu_vma_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
index b8c9011987f4..1dcfabf80453 100644
--- a/drivers/pci/mmap.c
+++ b/drivers/pci/mmap.c
@@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
 #endif
 
 static const struct vm_operations_struct pci_phys_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 43afbb7c5ab9..c9205a121007 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
 }
 
 static const struct vm_operations_struct uio_physical_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int uio_mmap_physical(struct vm_area_struct *vma)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index bc8f326be0ce..66d0cff6054e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1857,8 +1857,16 @@ int follow_pfn(struct vm_area_struct *vma, unsigned long address,
 	unsigned long *pfn);
 int follow_phys(struct vm_area_struct *vma, unsigned long address,
 		unsigned int flags, unsigned long *prot, resource_size_t *phys);
+#ifdef CONFIG_HAVE_IOREMAP_PROT
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write);
+#else
+int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+#endif
 
 extern void truncate_pagecache(struct inode *inode, loff_t new);
 extern void truncate_setsize(struct inode *inode, loff_t newsize);
diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..79b94db1bd5e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5437,9 +5437,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 		ret = get_user_pages_remote(mm, addr, 1,
 				gup_flags, &page, &vma, NULL);
 		if (ret <= 0) {
-#ifndef CONFIG_HAVE_IOREMAP_PROT
-			break;
-#else
 			/*
 			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
 			 * we can access using slightly different code.
@@ -5453,7 +5450,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 			if (ret <= 0)
 				break;
 			bytes = ret;
-#endif
 		} else {
 			bytes = len;
 			offset = addr & (PAGE_SIZE-1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-10  9:25 [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files Wang Wenhu
@ 2022-06-10  9:41 ` Greg Kroah-Hartman
  2022-06-10 10:07   ` Reply: " wenhu.wang
  2022-06-10 13:41 ` [PATCH v2] " Wang Wenhu
  1 sibling, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2022-06-10  9:41 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: Arnd Bergmann, Wu Hao, Tom Rix, Moritz Fischer, Xu Yilun,
	Bjorn Helgaas, Andrew Morton, open list,
	open list:FPGA DFL DRIVERS, open list:PCI SUBSYSTEM,
	open list:MEMORY MANAGEMENT, wenhu.wang

On Fri, Jun 10, 2022 at 02:25:18AM -0700, Wang Wenhu wrote:
> It is recommended in the "Conditional Compilation" chapter of kernel
> coding-style documentation that preprocessor conditionals should not
> be used in .c files wherever possible.
> 
> As for the micro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
> to eliminate it in .c files as we add a no-op function defination
> in the header file if the micro is not enabled.
> 
> The main trigger for this patch is an UIO driver series and as Greg
> commented we'd better not use such preprocessor contionals.
> See: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/
> For there is little work to do with the UIO driver, I try to push
> this commit independently.
> 
> Signed-off-by: Wang Wenhu <lonehugo@hotmail.com>
> ---
>  drivers/char/mem.c          | 2 --
>  drivers/fpga/dfl-afu-main.c | 2 --
>  drivers/pci/mmap.c          | 2 --
>  drivers/uio/uio.c           | 2 --
>  include/linux/mm.h          | 8 ++++++++
>  mm/memory.c                 | 4 ----
>  6 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 84ca98ed1dad..40186a441e38 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
>  #endif
>  
>  static const struct vm_operations_struct mmap_mem_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys
> -#endif
>  };
>  
>  static int mmap_mem(struct file *file, struct vm_area_struct *vma)
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 7f621e96d3b8..833e14806c7a 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  }
>  
>  static const struct vm_operations_struct afu_vma_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
> diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
> index b8c9011987f4..1dcfabf80453 100644
> --- a/drivers/pci/mmap.c
> +++ b/drivers/pci/mmap.c
> @@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
>  #endif
>  
>  static const struct vm_operations_struct pci_phys_vm_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index 43afbb7c5ab9..c9205a121007 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
>  }
>  
>  static const struct vm_operations_struct uio_physical_vm_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  static int uio_mmap_physical(struct vm_area_struct *vma)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index bc8f326be0ce..66d0cff6054e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1857,8 +1857,16 @@ int follow_pfn(struct vm_area_struct *vma, unsigned long address,
>  	unsigned long *pfn);
>  int follow_phys(struct vm_area_struct *vma, unsigned long address,
>  		unsigned int flags, unsigned long *prot, resource_size_t *phys);
> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>  int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  			void *buf, int len, int write);
> +#else
> +int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
> +			void *buf, int len, int write)

This needs to be an inline function, right?

Did you test build this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Reply: Re: [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-10  9:41 ` Greg Kroah-Hartman
@ 2022-06-10 10:07   ` wenhu.wang
  0 siblings, 0 replies; 5+ messages in thread
From: wenhu.wang @ 2022-06-10 10:07 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: Arnd Bergmann, Wu Hao, Tom Rix, Moritz Fischer, Xu Yilun,
	Bjorn Helgaas, Andrew Morton, open list,
	open list:FPGA DFL DRIVERS, open list:PCI SUBSYSTEM,
	open list:MEMORY MANAGEMENT, wenhu.wang

[-- Attachment #1: Type: text/plain, Size: 4384 bytes --]


> On Fri, Jun 10, 2022 at 02:25:18AM -0700, Wang Wenhu wrote: 
> > It is recommended in the "Conditional Compilation" chapter of kernel 
> > coding-style documentation that preprocessor conditionals should not 
> > be used in .c files wherever possible. 
> > 
> > As for the micro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance 
> > to eliminate it in .c files as we add a no-op function defination 
> > in the header file if the micro is not enabled. 
> > 
> > The main trigger for this patch is an UIO driver series and as Greg 
> > commented we'd better not use such preprocessor contionals. 
> > See: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/ 
> > For there is little work to do with the UIO driver, I try to push 
> > this commit independently. 
> > 
> > Signed-off-by: Wang Wenhu <lonehugo@hotmail.com> 
> > --- 
> >  drivers/char/mem.c          | 2 -- 
> >  drivers/fpga/dfl-afu-main.c | 2 -- 
> >  drivers/pci/mmap.c          | 2 -- 
> >  drivers/uio/uio.c           | 2 -- 
> >  include/linux/mm.h          | 8 ++++++++ 
> >  mm/memory.c                 | 4 ---- 
> >  6 files changed, 8 insertions(+), 12 deletions(-) 
> > 
> > diff --git a/drivers/char/mem.c b/drivers/char/mem.c 
> > index 84ca98ed1dad..40186a441e38 100644 
> > --- a/drivers/char/mem.c 
> > +++ b/drivers/char/mem.c 
> > @@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma) 
> >  #endif 
> >  
> >  static const struct vm_operations_struct mmap_mem_ops = { 
> > -#ifdef CONFIG_HAVE_IOREMAP_PROT 
> >  .access = generic_access_phys 
> > -#endif 
> >  }; 
> >  
> >  static int mmap_mem(struct file *file, struct vm_area_struct *vma) 
> > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c 
> > index 7f621e96d3b8..833e14806c7a 100644 
> > --- a/drivers/fpga/dfl-afu-main.c 
> > +++ b/drivers/fpga/dfl-afu-main.c 
> > @@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) 
> >  } 
> >  
> >  static const struct vm_operations_struct afu_vma_ops = { 
> > -#ifdef CONFIG_HAVE_IOREMAP_PROT 
> >  .access = generic_access_phys, 
> > -#endif 
> >  }; 
> >  
> >  static int afu_mmap(struct file *filp, struct vm_area_struct *vma) 
> > diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c 
> > index b8c9011987f4..1dcfabf80453 100644 
> > --- a/drivers/pci/mmap.c 
> > +++ b/drivers/pci/mmap.c 
> > @@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar, 
> >  #endif 
> >  
> >  static const struct vm_operations_struct pci_phys_vm_ops = { 
> > -#ifdef CONFIG_HAVE_IOREMAP_PROT 
> >  .access = generic_access_phys, 
> > -#endif 
> >  }; 
> >  
> >  int pci_mmap_resource_range(struct pci_dev *pdev, int bar, 
> > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c 
> > index 43afbb7c5ab9..c9205a121007 100644 
> > --- a/drivers/uio/uio.c 
> > +++ b/drivers/uio/uio.c 
> > @@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma) 
> >  } 
> >  
> >  static const struct vm_operations_struct uio_physical_vm_ops = { 
> > -#ifdef CONFIG_HAVE_IOREMAP_PROT 
> >  .access = generic_access_phys, 
> > -#endif 
> >  }; 
> >  
> >  static int uio_mmap_physical(struct vm_area_struct *vma) 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h 
> > index bc8f326be0ce..66d0cff6054e 100644 
> > --- a/include/linux/mm.h 
> > +++ b/include/linux/mm.h 
> > @@ -1857,8 +1857,16 @@ int follow_pfn(struct vm_area_struct *vma, unsigned long address, 
> >  unsigned long *pfn); 
> >  int follow_phys(struct vm_area_struct *vma, unsigned long address, 
> >  unsigned int flags, unsigned long *prot, resource_size_t *phys); 
> > +#ifdef CONFIG_HAVE_IOREMAP_PROT 
> >  int generic_access_phys(struct vm_area_struct *vma, unsigned long addr, 
> >  void *buf, int len, int write); 
> > +#else 
> > +int generic_access_phys(struct vm_area_struct *vma, unsigned long addr, 
> > + void *buf, int len, int write) 
>
> This needs to be an inline function, right? 
>
> Did you test build this? 
>
> thanks, 
>
> greg k-h 
Surely it should have been static inline orelse undefine error happens.
I tested with the micro enabled on ppc, and disabled on arm64,
but I missed up configurations on arm testing code that it's still on.
I will re-confirm it and update the patch later.

thanks,
Wenhu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-10  9:25 [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files Wang Wenhu
  2022-06-10  9:41 ` Greg Kroah-Hartman
@ 2022-06-10 13:41 ` Wang Wenhu
  2022-06-10 14:43   ` Bjorn Helgaas
  1 sibling, 1 reply; 5+ messages in thread
From: Wang Wenhu @ 2022-06-10 13:41 UTC (permalink / raw)
  To: lonehugo, akpm, arnd, bhelgaas, gregkh, hao.wu, linux-fpga, mdf,
	trix, yilun.xu
  Cc: linux-kernel, linux-mm, linux-pci, wenhu.wang

It is recommended in the "Conditional Compilation" chapter of kernel
coding-style documentation that preprocessor conditionals should not
be used in .c files wherever possible.

As for the micro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
to eliminate it in .c files as we add a no-op function defination
in the header file if the micro is not enabled.

The main trigger for this patch is an UIO driver series and as Greg
commented we'd better not use such preprocessor contionals.
See: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/
For there is little work to do with the UIO driver, I try to push
this commit independently.

Signed-off-by: Wang Wenhu <lonehugo@hotmail.com>
---
v2: specify no-op function definition with static inline
---
 drivers/char/mem.c          | 2 --
 drivers/fpga/dfl-afu-main.c | 2 --
 drivers/pci/mmap.c          | 2 --
 drivers/uio/uio.c           | 2 --
 include/linux/mm.h          | 8 ++++++++
 mm/memory.c                 | 4 ----
 6 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 84ca98ed1dad..40186a441e38 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
 #endif
 
 static const struct vm_operations_struct mmap_mem_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys
-#endif
 };
 
 static int mmap_mem(struct file *file, struct vm_area_struct *vma)
diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 7f621e96d3b8..833e14806c7a 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 }
 
 static const struct vm_operations_struct afu_vma_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
index b8c9011987f4..1dcfabf80453 100644
--- a/drivers/pci/mmap.c
+++ b/drivers/pci/mmap.c
@@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
 #endif
 
 static const struct vm_operations_struct pci_phys_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 43afbb7c5ab9..c9205a121007 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
 }
 
 static const struct vm_operations_struct uio_physical_vm_ops = {
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 	.access = generic_access_phys,
-#endif
 };
 
 static int uio_mmap_physical(struct vm_area_struct *vma)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index bc8f326be0ce..60c183dce5ea 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1857,8 +1857,16 @@ int follow_pfn(struct vm_area_struct *vma, unsigned long address,
 	unsigned long *pfn);
 int follow_phys(struct vm_area_struct *vma, unsigned long address,
 		unsigned int flags, unsigned long *prot, resource_size_t *phys);
+#ifdef CONFIG_HAVE_IOREMAP_PROT
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write);
+#else
+static inline int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
+			void *buf, int len, int write)
+{
+	return 0;
+}
+#endif
 
 extern void truncate_pagecache(struct inode *inode, loff_t new);
 extern void truncate_setsize(struct inode *inode, loff_t newsize);
diff --git a/mm/memory.c b/mm/memory.c
index 7a089145cad4..79b94db1bd5e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5437,9 +5437,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 		ret = get_user_pages_remote(mm, addr, 1,
 				gup_flags, &page, &vma, NULL);
 		if (ret <= 0) {
-#ifndef CONFIG_HAVE_IOREMAP_PROT
-			break;
-#else
 			/*
 			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
 			 * we can access using slightly different code.
@@ -5453,7 +5450,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
 			if (ret <= 0)
 				break;
 			bytes = ret;
-#endif
 		} else {
 			bytes = len;
 			offset = addr & (PAGE_SIZE-1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files
  2022-06-10 13:41 ` [PATCH v2] " Wang Wenhu
@ 2022-06-10 14:43   ` Bjorn Helgaas
  0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2022-06-10 14:43 UTC (permalink / raw)
  To: Wang Wenhu
  Cc: akpm, arnd, bhelgaas, gregkh, hao.wu, linux-fpga, mdf, trix,
	yilun.xu, linux-kernel, linux-mm, linux-pci, wenhu.wang

On Fri, Jun 10, 2022 at 06:41:54AM -0700, Wang Wenhu wrote:
> It is recommended in the "Conditional Compilation" chapter of kernel
> coding-style documentation that preprocessor conditionals should not
> be used in .c files wherever possible.
> 
> As for the micro CONFIG_HAVE_IOREMAP_PROT, now it's a proper chance
> to eliminate it in .c files as we add a no-op function defination
> in the header file if the micro is not enabled.

s/micro/macro/ (twice)
s/defination/definition/

> The main trigger for this patch is an UIO driver series and as Greg
> commented we'd better not use such preprocessor contionals.

s/contionals/conditionals/

> See: https://lore.kernel.org/lkml/YqHy1uXwCLlJmftr@kroah.com/
> For there is little work to do with the UIO driver, I try to push
> this commit independently.
> 
> Signed-off-by: Wang Wenhu <lonehugo@hotmail.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# drivers/pci/

Thanks for cleaning this up!

> ---
> v2: specify no-op function definition with static inline
> ---
>  drivers/char/mem.c          | 2 --
>  drivers/fpga/dfl-afu-main.c | 2 --
>  drivers/pci/mmap.c          | 2 --
>  drivers/uio/uio.c           | 2 --
>  include/linux/mm.h          | 8 ++++++++
>  mm/memory.c                 | 4 ----
>  6 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index 84ca98ed1dad..40186a441e38 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -354,9 +354,7 @@ static inline int private_mapping_ok(struct vm_area_struct *vma)
>  #endif
>  
>  static const struct vm_operations_struct mmap_mem_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys
> -#endif
>  };
>  
>  static int mmap_mem(struct file *file, struct vm_area_struct *vma)
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 7f621e96d3b8..833e14806c7a 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -797,9 +797,7 @@ static long afu_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  }
>  
>  static const struct vm_operations_struct afu_vma_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  static int afu_mmap(struct file *filp, struct vm_area_struct *vma)
> diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c
> index b8c9011987f4..1dcfabf80453 100644
> --- a/drivers/pci/mmap.c
> +++ b/drivers/pci/mmap.c
> @@ -35,9 +35,7 @@ int pci_mmap_page_range(struct pci_dev *pdev, int bar,
>  #endif
>  
>  static const struct vm_operations_struct pci_phys_vm_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index 43afbb7c5ab9..c9205a121007 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -719,9 +719,7 @@ static int uio_mmap_logical(struct vm_area_struct *vma)
>  }
>  
>  static const struct vm_operations_struct uio_physical_vm_ops = {
> -#ifdef CONFIG_HAVE_IOREMAP_PROT
>  	.access = generic_access_phys,
> -#endif
>  };
>  
>  static int uio_mmap_physical(struct vm_area_struct *vma)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index bc8f326be0ce..60c183dce5ea 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1857,8 +1857,16 @@ int follow_pfn(struct vm_area_struct *vma, unsigned long address,
>  	unsigned long *pfn);
>  int follow_phys(struct vm_area_struct *vma, unsigned long address,
>  		unsigned int flags, unsigned long *prot, resource_size_t *phys);
> +#ifdef CONFIG_HAVE_IOREMAP_PROT
>  int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
>  			void *buf, int len, int write);
> +#else
> +static inline int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
> +			void *buf, int len, int write)
> +{
> +	return 0;
> +}
> +#endif
>  
>  extern void truncate_pagecache(struct inode *inode, loff_t new);
>  extern void truncate_setsize(struct inode *inode, loff_t newsize);
> diff --git a/mm/memory.c b/mm/memory.c
> index 7a089145cad4..79b94db1bd5e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5437,9 +5437,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
>  		ret = get_user_pages_remote(mm, addr, 1,
>  				gup_flags, &page, &vma, NULL);
>  		if (ret <= 0) {
> -#ifndef CONFIG_HAVE_IOREMAP_PROT
> -			break;
> -#else
>  			/*
>  			 * Check if this is a VM_IO | VM_PFNMAP VMA, which
>  			 * we can access using slightly different code.
> @@ -5453,7 +5450,6 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
>  			if (ret <= 0)
>  				break;
>  			bytes = ret;
> -#endif
>  		} else {
>  			bytes = len;
>  			offset = addr & (PAGE_SIZE-1);
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-06-10 14:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10  9:25 [PATCH] mm: eliminate ifdef of HAVE_IOREMAP_PROT in .c files Wang Wenhu
2022-06-10  9:41 ` Greg Kroah-Hartman
2022-06-10 10:07   ` Reply: " wenhu.wang
2022-06-10 13:41 ` [PATCH v2] " Wang Wenhu
2022-06-10 14:43   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).