diff mbox series

[v4] mm: Create the new vm_fault_t type

Message ID 20190108183041.GA12137@jordon-HP-15-Notebook-PC (mailing list archive)
State New, archived
Headers show
Series [v4] mm: Create the new vm_fault_t type | expand

Commit Message

Souptick Joarder Jan. 8, 2019, 6:30 p.m. UTC
Page fault handlers are supposed to return VM_FAULT codes,
but some drivers/file systems mistakenly return error
numbers. Now that all drivers/file systems have been converted
to use the vm_fault_t return type, change the type definition
to no longer be compatible with 'int'. By making it an unsigned
int, the function prototype becomes incompatible with a function
which returns int. Sparse will detect any attempts to return a
value which is not a VM_FAULT code.

VM_FAULT_SET_HINDEX and VM_FAULT_GET_HINDEX values are changed
to avoid conflict with other VM_FAULT codes.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Reviewed-by: William Kucharski <william.kucharski@oracle.com>
Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
---
v2: Updated the change log and corrected the document part.
    name added to the enum that kernel-doc able to parse it.

v3: Corrected the documentation.

v4: Reviewed by William and Mike ( for docs part).

 include/linux/mm.h       | 46 ------------------------------
 include/linux/mm_types.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 47 deletions(-)

Comments

Matthew Wilcox (Oracle) Jan. 8, 2019, 6:29 p.m. UTC | #1
On Wed, Jan 09, 2019 at 12:00:41AM +0530, Souptick Joarder wrote:
> Page fault handlers are supposed to return VM_FAULT codes,
> but some drivers/file systems mistakenly return error
> numbers. Now that all drivers/file systems have been converted
> to use the vm_fault_t return type, change the type definition
> to no longer be compatible with 'int'. By making it an unsigned
> int, the function prototype becomes incompatible with a function
> which returns int. Sparse will detect any attempts to return a
> value which is not a VM_FAULT code.
> 
> VM_FAULT_SET_HINDEX and VM_FAULT_GET_HINDEX values are changed
> to avoid conflict with other VM_FAULT codes.
> 
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>
> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

Reviewed-by: Matthew Wilcox <willy@infradead.org>
kernel test robot Jan. 9, 2019, 11:50 a.m. UTC | #2
Hi Souptick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc1 next-20190109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/mm-Create-the-new-vm_fault_t-type/20190109-154216
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> arch/x86/mm/fault.c:1051:39: warning: restricted vm_fault_t degrades to integer
   arch/x86/mm/fault.c:1057:29: warning: restricted vm_fault_t degrades to integer
   arch/x86/mm/fault.c:1059:29: warning: restricted vm_fault_t degrades to integer
>> arch/x86/mm/fault.c:1094:62: warning: incorrect type in argument 4 (different base types)
   arch/x86/mm/fault.c:1094:62:    expected unsigned int fault
   arch/x86/mm/fault.c:1094:62:    got restricted vm_fault_t [usertype] fault

vim +1051 arch/x86/mm/fault.c

92181f190 Nick Piggin       2009-01-20  1031  
2d4a71676 Ingo Molnar       2009-02-20  1032  static void
a6e04aa92 Andi Kleen        2009-09-16  1033  do_sigbus(struct pt_regs *regs, unsigned long error_code, unsigned long address,
27274f731 Eric W. Biederman 2018-09-18  1034  	  unsigned int fault)
92181f190 Nick Piggin       2009-01-20  1035  {
92181f190 Nick Piggin       2009-01-20  1036  	struct task_struct *tsk = current;
92181f190 Nick Piggin       2009-01-20  1037  
2d4a71676 Ingo Molnar       2009-02-20  1038  	/* Kernel mode? Handle exceptions or die: */
1067f0309 Ricardo Neri      2017-10-27  1039  	if (!(error_code & X86_PF_USER)) {
4fc349011 Andy Lutomirski   2011-11-07  1040  		no_context(regs, error_code, address, SIGBUS, BUS_ADRERR);
960545691 Linus Torvalds    2010-08-13  1041  		return;
960545691 Linus Torvalds    2010-08-13  1042  	}
2d4a71676 Ingo Molnar       2009-02-20  1043  
cd1b68f08 Ingo Molnar       2009-02-20  1044  	/* User-space => ok to do another page fault: */
92181f190 Nick Piggin       2009-01-20  1045  	if (is_prefetch(regs, error_code, address))
92181f190 Nick Piggin       2009-01-20  1046  		return;
2d4a71676 Ingo Molnar       2009-02-20  1047  
e49d3cbef Andy Lutomirski   2018-11-19  1048  	set_signal_archinfo(address, error_code);
2d4a71676 Ingo Molnar       2009-02-20  1049  
a6e04aa92 Andi Kleen        2009-09-16  1050  #ifdef CONFIG_MEMORY_FAILURE
f672b49b0 Andi Kleen        2010-09-27 @1051  	if (fault & (VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) {
40e553946 Eric W. Biederman 2018-01-19  1052  		unsigned lsb = 0;
40e553946 Eric W. Biederman 2018-01-19  1053  
40e553946 Eric W. Biederman 2018-01-19  1054  		pr_err(
a6e04aa92 Andi Kleen        2009-09-16  1055  	"MCE: Killing %s:%d due to hardware memory corruption fault at %lx\n",
a6e04aa92 Andi Kleen        2009-09-16  1056  			tsk->comm, tsk->pid, address);
40e553946 Eric W. Biederman 2018-01-19  1057  		if (fault & VM_FAULT_HWPOISON_LARGE)
40e553946 Eric W. Biederman 2018-01-19  1058  			lsb = hstate_index_to_shift(VM_FAULT_GET_HINDEX(fault));
40e553946 Eric W. Biederman 2018-01-19  1059  		if (fault & VM_FAULT_HWPOISON)
40e553946 Eric W. Biederman 2018-01-19  1060  			lsb = PAGE_SHIFT;
40e553946 Eric W. Biederman 2018-01-19  1061  		force_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, tsk);
40e553946 Eric W. Biederman 2018-01-19  1062  		return;
a6e04aa92 Andi Kleen        2009-09-16  1063  	}
a6e04aa92 Andi Kleen        2009-09-16  1064  #endif
b4fd52f25 Eric W. Biederman 2018-09-18  1065  	force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk);
92181f190 Nick Piggin       2009-01-20  1066  }
92181f190 Nick Piggin       2009-01-20  1067  
3a13c4d76 Johannes Weiner   2013-09-12  1068  static noinline void
2d4a71676 Ingo Molnar       2009-02-20  1069  mm_fault_error(struct pt_regs *regs, unsigned long error_code,
25c102d80 Eric W. Biederman 2018-09-18  1070  	       unsigned long address, vm_fault_t fault)
92181f190 Nick Piggin       2009-01-20  1071  {
1067f0309 Ricardo Neri      2017-10-27  1072  	if (fatal_signal_pending(current) && !(error_code & X86_PF_USER)) {
4fc349011 Andy Lutomirski   2011-11-07  1073  		no_context(regs, error_code, address, 0, 0);
3a13c4d76 Johannes Weiner   2013-09-12  1074  		return;
b80ef10e8 KOSAKI Motohiro   2011-05-26  1075  	}
b80ef10e8 KOSAKI Motohiro   2011-05-26  1076  
2d4a71676 Ingo Molnar       2009-02-20  1077  	if (fault & VM_FAULT_OOM) {
f86268549 Andrey Vagin      2011-03-09  1078  		/* Kernel mode? Handle exceptions or die: */
1067f0309 Ricardo Neri      2017-10-27  1079  		if (!(error_code & X86_PF_USER)) {
4fc349011 Andy Lutomirski   2011-11-07  1080  			no_context(regs, error_code, address,
4fc349011 Andy Lutomirski   2011-11-07  1081  				   SIGSEGV, SEGV_MAPERR);
3a13c4d76 Johannes Weiner   2013-09-12  1082  			return;
f86268549 Andrey Vagin      2011-03-09  1083  		}
f86268549 Andrey Vagin      2011-03-09  1084  
c2d23f919 David Rientjes    2012-12-12  1085  		/*
c2d23f919 David Rientjes    2012-12-12  1086  		 * We ran out of memory, call the OOM killer, and return the
c2d23f919 David Rientjes    2012-12-12  1087  		 * userspace (which will retry the fault, or kill us if we got
c2d23f919 David Rientjes    2012-12-12  1088  		 * oom-killed):
c2d23f919 David Rientjes    2012-12-12  1089  		 */
c2d23f919 David Rientjes    2012-12-12  1090  		pagefault_out_of_memory();
2d4a71676 Ingo Molnar       2009-02-20  1091  	} else {
f672b49b0 Andi Kleen        2010-09-27  1092  		if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON|
f672b49b0 Andi Kleen        2010-09-27  1093  			     VM_FAULT_HWPOISON_LARGE))
27274f731 Eric W. Biederman 2018-09-18 @1094  			do_sigbus(regs, error_code, address, fault);
33692f275 Linus Torvalds    2015-01-29  1095  		else if (fault & VM_FAULT_SIGSEGV)
768fd9c69 Eric W. Biederman 2018-09-18  1096  			bad_area_nosemaphore(regs, error_code, address);
92181f190 Nick Piggin       2009-01-20  1097  		else
92181f190 Nick Piggin       2009-01-20  1098  			BUG();
92181f190 Nick Piggin       2009-01-20  1099  	}
2d4a71676 Ingo Molnar       2009-02-20  1100  }
92181f190 Nick Piggin       2009-01-20  1101  

:::::: The code at line 1051 was first introduced by commit
:::::: f672b49b07a4a152fc4251f2aec6b4d05164c4cd x86: HWPOISON: Report correct address granuality for huge hwpoison faults

:::::: TO: Andi Kleen <ak@linux.intel.com>
:::::: CC: Andi Kleen <ak@linux.intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Jan. 9, 2019, 3:13 p.m. UTC | #3
Hi Souptick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0-rc1 next-20190109]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Souptick-Joarder/mm-Create-the-new-vm_fault_t-type/20190109-154216
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> kernel/memremap.c:46:34: warning: incorrect type in return expression (different base types)
   kernel/memremap.c:46:34:    expected restricted vm_fault_t
   kernel/memremap.c:46:34:    got int

vim +46 kernel/memremap.c

9476df7d Dan Williams     2016-01-15  19  
5042db43 Jérôme Glisse    2017-09-08  20  #if IS_ENABLED(CONFIG_DEVICE_PRIVATE)
2b740303 Souptick Joarder 2018-08-23  21  vm_fault_t device_private_entry_fault(struct vm_area_struct *vma,
5042db43 Jérôme Glisse    2017-09-08  22  		       unsigned long addr,
5042db43 Jérôme Glisse    2017-09-08  23  		       swp_entry_t entry,
5042db43 Jérôme Glisse    2017-09-08  24  		       unsigned int flags,
5042db43 Jérôme Glisse    2017-09-08  25  		       pmd_t *pmdp)
5042db43 Jérôme Glisse    2017-09-08  26  {
5042db43 Jérôme Glisse    2017-09-08  27  	struct page *page = device_private_entry_to_page(entry);
063a7d1d Dan Williams     2018-12-28  28  	struct hmm_devmem *devmem;
063a7d1d Dan Williams     2018-12-28  29  
063a7d1d Dan Williams     2018-12-28  30  	devmem = container_of(page->pgmap, typeof(*devmem), pagemap);
5042db43 Jérôme Glisse    2017-09-08  31  
5042db43 Jérôme Glisse    2017-09-08  32  	/*
5042db43 Jérôme Glisse    2017-09-08  33  	 * The page_fault() callback must migrate page back to system memory
5042db43 Jérôme Glisse    2017-09-08  34  	 * so that CPU can access it. This might fail for various reasons
5042db43 Jérôme Glisse    2017-09-08  35  	 * (device issue, device was unsafely unplugged, ...). When such
5042db43 Jérôme Glisse    2017-09-08  36  	 * error conditions happen, the callback must return VM_FAULT_SIGBUS.
5042db43 Jérôme Glisse    2017-09-08  37  	 *
5042db43 Jérôme Glisse    2017-09-08  38  	 * Note that because memory cgroup charges are accounted to the device
5042db43 Jérôme Glisse    2017-09-08  39  	 * memory, this should never fail because of memory restrictions (but
5042db43 Jérôme Glisse    2017-09-08  40  	 * allocation of regular system page might still fail because we are
5042db43 Jérôme Glisse    2017-09-08  41  	 * out of memory).
5042db43 Jérôme Glisse    2017-09-08  42  	 *
5042db43 Jérôme Glisse    2017-09-08  43  	 * There is a more in-depth description of what that callback can and
5042db43 Jérôme Glisse    2017-09-08  44  	 * cannot do, in include/linux/memremap.h
5042db43 Jérôme Glisse    2017-09-08  45  	 */
063a7d1d Dan Williams     2018-12-28 @46  	return devmem->page_fault(vma, addr, page, flags, pmdp);
5042db43 Jérôme Glisse    2017-09-08  47  }
5042db43 Jérôme Glisse    2017-09-08  48  EXPORT_SYMBOL(device_private_entry_fault);
5042db43 Jérôme Glisse    2017-09-08  49  #endif /* CONFIG_DEVICE_PRIVATE */
5042db43 Jérôme Glisse    2017-09-08  50  

:::::: The code at line 46 was first introduced by commit
:::::: 063a7d1d3623db31ca5d2309cab6030ebf93b72f mm/hmm: fix memremap.h, move dev_page_fault_t callback to hmm

:::::: TO: Dan Williams <dan.j.williams@intel.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index fcf9cc9..511a3ce 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1267,52 +1267,6 @@  static inline void clear_page_pfmemalloc(struct page *page)
 }
 
 /*
- * Different kinds of faults, as returned by handle_mm_fault().
- * Used to decide whether a process gets delivered SIGBUS or
- * just gets major/minor fault counters bumped up.
- */
-
-#define VM_FAULT_OOM	0x0001
-#define VM_FAULT_SIGBUS	0x0002
-#define VM_FAULT_MAJOR	0x0004
-#define VM_FAULT_WRITE	0x0008	/* Special case for get_user_pages */
-#define VM_FAULT_HWPOISON 0x0010	/* Hit poisoned small page */
-#define VM_FAULT_HWPOISON_LARGE 0x0020  /* Hit poisoned large page. Index encoded in upper bits */
-#define VM_FAULT_SIGSEGV 0x0040
-
-#define VM_FAULT_NOPAGE	0x0100	/* ->fault installed the pte, not return page */
-#define VM_FAULT_LOCKED	0x0200	/* ->fault locked the returned page */
-#define VM_FAULT_RETRY	0x0400	/* ->fault blocked, must retry */
-#define VM_FAULT_FALLBACK 0x0800	/* huge page fault failed, fall back to small */
-#define VM_FAULT_DONE_COW   0x1000	/* ->fault has fully handled COW */
-#define VM_FAULT_NEEDDSYNC  0x2000	/* ->fault did not modify page tables
-					 * and needs fsync() to complete (for
-					 * synchronous page faults in DAX) */
-
-#define VM_FAULT_ERROR	(VM_FAULT_OOM | VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | \
-			 VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE | \
-			 VM_FAULT_FALLBACK)
-
-#define VM_FAULT_RESULT_TRACE \
-	{ VM_FAULT_OOM,			"OOM" }, \
-	{ VM_FAULT_SIGBUS,		"SIGBUS" }, \
-	{ VM_FAULT_MAJOR,		"MAJOR" }, \
-	{ VM_FAULT_WRITE,		"WRITE" }, \
-	{ VM_FAULT_HWPOISON,		"HWPOISON" }, \
-	{ VM_FAULT_HWPOISON_LARGE,	"HWPOISON_LARGE" }, \
-	{ VM_FAULT_SIGSEGV,		"SIGSEGV" }, \
-	{ VM_FAULT_NOPAGE,		"NOPAGE" }, \
-	{ VM_FAULT_LOCKED,		"LOCKED" }, \
-	{ VM_FAULT_RETRY,		"RETRY" }, \
-	{ VM_FAULT_FALLBACK,		"FALLBACK" }, \
-	{ VM_FAULT_DONE_COW,		"DONE_COW" }, \
-	{ VM_FAULT_NEEDDSYNC,		"NEEDDSYNC" }
-
-/* Encode hstate index for a hwpoisoned large page */
-#define VM_FAULT_SET_HINDEX(x) ((x) << 12)
-#define VM_FAULT_GET_HINDEX(x) (((x) >> 12) & 0xf)
-
-/*
  * Can be called by the pagefault handler when it gets a VM_FAULT_OOM.
  */
 extern void pagefault_out_of_memory(void);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ed8f62..cb25016 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -22,7 +22,6 @@ 
 #endif
 #define AT_VECTOR_SIZE (2*(AT_VECTOR_SIZE_ARCH + AT_VECTOR_SIZE_BASE + 1))
 
-typedef int vm_fault_t;
 
 struct address_space;
 struct mem_cgroup;
@@ -609,6 +608,78 @@  static inline bool mm_tlb_flush_nested(struct mm_struct *mm)
 
 struct vm_fault;
 
+/**
+ * typedef vm_fault_t - Return type for page fault handlers.
+ *
+ * Page fault handlers return a bitmask of %VM_FAULT values.
+ */
+typedef __bitwise unsigned int vm_fault_t;
+
+/**
+ * enum vm_fault_reason - Page fault handlers return a bitmask of
+ * these values to tell the core VM what happened when handling the
+ * fault. Used to decide whether a process gets delivered SIGBUS or
+ * just gets major/minor fault counters bumped up.
+ *
+ * @VM_FAULT_OOM:		Out Of Memory
+ * @VM_FAULT_SIGBUS:		Bad access
+ * @VM_FAULT_MAJOR:		Page read from storage
+ * @VM_FAULT_WRITE:		Special case for get_user_pages
+ * @VM_FAULT_HWPOISON:		Hit poisoned small page
+ * @VM_FAULT_HWPOISON_LARGE:	Hit poisoned large page. Index encoded
+ *				in upper bits
+ * @VM_FAULT_SIGSEGV:		segmentation fault
+ * @VM_FAULT_NOPAGE:		->fault installed the pte, not return page
+ * @VM_FAULT_LOCKED:		->fault locked the returned page
+ * @VM_FAULT_RETRY:		->fault blocked, must retry
+ * @VM_FAULT_FALLBACK:		huge page fault failed, fall back to small
+ * @VM_FAULT_DONE_COW:		->fault has fully handled COW
+ * @VM_FAULT_NEEDDSYNC:		->fault did not modify page tables and needs
+ *				fsync() to complete (for synchronous page faults
+ *				in DAX)
+ * @VM_FAULT_HINDEX_MASK:	mask HINDEX value
+ *
+ */
+enum vm_fault_reason {
+	VM_FAULT_OOM            = (__force vm_fault_t)0x000001,
+	VM_FAULT_SIGBUS         = (__force vm_fault_t)0x000002,
+	VM_FAULT_MAJOR          = (__force vm_fault_t)0x000004,
+	VM_FAULT_WRITE          = (__force vm_fault_t)0x000008,
+	VM_FAULT_HWPOISON       = (__force vm_fault_t)0x000010,
+	VM_FAULT_HWPOISON_LARGE = (__force vm_fault_t)0x000020,
+	VM_FAULT_SIGSEGV        = (__force vm_fault_t)0x000040,
+	VM_FAULT_NOPAGE         = (__force vm_fault_t)0x000100,
+	VM_FAULT_LOCKED         = (__force vm_fault_t)0x000200,
+	VM_FAULT_RETRY          = (__force vm_fault_t)0x000400,
+	VM_FAULT_FALLBACK       = (__force vm_fault_t)0x000800,
+	VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000,
+	VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000,
+	VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
+};
+
+/* Encode hstate index for a hwpoisoned large page */
+#define VM_FAULT_SET_HINDEX(x) ((__force vm_fault_t)((x) << 16))
+#define VM_FAULT_GET_HINDEX(x) (((x) >> 16) & 0xf)
+
+#define VM_FAULT_ERROR (VM_FAULT_OOM | VM_FAULT_SIGBUS |	\
+			VM_FAULT_SIGSEGV | VM_FAULT_HWPOISON |	\
+			VM_FAULT_HWPOISON_LARGE | VM_FAULT_FALLBACK)
+
+#define VM_FAULT_RESULT_TRACE \
+	{ VM_FAULT_OOM,                 "OOM" },	\
+	{ VM_FAULT_SIGBUS,              "SIGBUS" },	\
+	{ VM_FAULT_MAJOR,               "MAJOR" },	\
+	{ VM_FAULT_WRITE,               "WRITE" },	\
+	{ VM_FAULT_HWPOISON,            "HWPOISON" },	\
+	{ VM_FAULT_HWPOISON_LARGE,      "HWPOISON_LARGE" },	\
+	{ VM_FAULT_SIGSEGV,             "SIGSEGV" },	\
+	{ VM_FAULT_NOPAGE,              "NOPAGE" },	\
+	{ VM_FAULT_LOCKED,              "LOCKED" },	\
+	{ VM_FAULT_RETRY,               "RETRY" },	\
+	{ VM_FAULT_FALLBACK,            "FALLBACK" },	\
+	{ VM_FAULT_DONE_COW,            "DONE_COW" },	\
+	{ VM_FAULT_NEEDDSYNC,           "NEEDDSYNC" }
+
 struct vm_special_mapping {
 	const char *name;	/* The name, e.g. "[vdso]". */