It is currently Tue, 14 Jul 2020 02:04:23 GMT



 
Author Message
 remove magic numbers for fault return codes
The return codes from fault handling functions are open-coded in
memory.c and arch/i386/mm/fault.c.

This patch removes them in favor of defining integer constants with
mnemonic names and using them as the return codes and in dispatches
on the return values from handle_mm_fault(), and also removing some of
the associated comments explaining what the magic numbers mean at
various points where they were formerly used. In addition two redundant
variables were removed, a comment was added to the bad_wp_page: label
to explain to onlookers more of what the printk() there means, and
failure to match the return code with a value in the various switch()
statements on the return code was flagged as a BUG() at various points.

From what I saw in page_alloc.c last time, since these hit the same
lines, they might as well all go out in one batch. I am, of course,
willing to adjust/separate various bits as desired.

Cheers,
Bill

===== include/linux/mm.h 1.55 vs edited =====
--- 1.55/include/linux/mm.h     Sat May 25 16:25:47 2002
+++ edited/include/linux/mm.h   Fri Jun  7 15:59:07 2002
@@ -305,6 +305,16 @@
 #define NOPAGE_SIGBUS  (NULL)
 #define NOPAGE_OOM     ((struct page *) (-1))

+/*
+ * 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   (-1)
+#define VM_FAULT_SIGBUS        0
+#define VM_FAULT_MINOR 1
+#define VM_FAULT_MAJOR 2
+
 /* The array of struct pages */
 extern struct page *mem_map;

===== mm/memory.c 1.70 vs edited =====
--- 1.70/mm/memory.c    Fri May 31 18:18:07 2002
+++ edited/mm/memory.c  Fri Jun  7 15:19:49 2002
@@ -514,18 +514,18 @@
                        while (!(map = follow_page(mm, start, write))) {
                                spin_unlock(&mm->page_table_lock);
                                switch (handle_mm_fault(mm, vma, start, write)) {
-                               case 1:
+                               case VM_FAULT_MINOR:
                                        tsk->min_flt++;
                                        break;
-                               case 2:
+                               case VM_FAULT_MAJOR:
                                        tsk->maj_flt++;
                                        break;
-                               case 0:
-                                       if (i) return i;
-                                       return -EFAULT;
+                               case VM_FAULT_SIGBUS:
+                                       return i ? i : -EFAULT;
+                               case VM_FAULT_OOM:
+                                       return i ? i : -ENOMEM;
                                default:
-                                       if (i) return i;
-                                       return -ENOMEM;
+                                       BUG();
                                }
                                spin_lock(&mm->page_table_lock);
                        }
@@ -982,7 +982,7 @@
                        establish_pte(vma, address, page_table, pte_mkyoung(pte_mkdirty(pte_mkwrite(pte))));
                        pte_unmap(page_table);
                        spin_unlock(&mm->page_table_lock);
-                       return 1;       /* Minor fault */
+                       return VM_FAULT_MINOR;
                }
        }
        pte_unmap(page_table);
@@ -1016,16 +1016,21 @@
        spin_unlock(&mm->page_table_lock);
        page_cache_release(new_page);
        page_cache_release(old_page);
-       return 1;       /* Minor fault */
+       return VM_FAULT_MINOR;

 bad_wp_page:
        pte_unmap(page_table);
        spin_unlock(&mm->page_table_lock);
        printk(KERN_ERR "do_wp_page: bogus page at address %08lx\n", address);
-       return -1;
+       /*
+        * This should really halt the system so it can be debugged or
+        * at least the kernel stops what it's doing before it corrupts
+        * data, but for the moment just pretend this is OOM.
+        */
+       return VM_FAULT_OOM;
 no_mem:
        page_cache_release(old_page);
-       return -1;
+       return VM_FAULT_OOM;
 }

 static void vmtruncate_list(list_t *head, unsigned long pgoff)
@@ -1149,7 +1154,7 @@
        struct page *page;
        swp_entry_t entry = pte_to_swp_entry(orig_pte);
        pte_t pte;
-       int ret = 1;
+       int ret = VM_FAULT_MINOR;

        pte_unmap(page_table);
        spin_unlock(&mm->page_table_lock);
@@ -1162,17 +1167,19 @@
                         * Back out if somebody else faulted in this pte while
                         * we released the page table lock.
                         */
-                       int retval;
                        spin_lock(&mm->page_table_lock);
                        page_table = pte_offset_map(pmd, address);
-                       retval = pte_same(*page_table, orig_pte) ? -1 : 1;
+                       if (pte_same(*page_table, orig_pte))
+                               ret = VM_FAULT_OOM;
+                       else
+                               ret = VM_FAULT_MINOR;
                        pte_unmap(page_table);
                        spin_unlock(&mm->page_table_lock);
-                       return retval;
+                       return ret;
                }

                /* Had to read the page from swap area: Major fault */
-               ret = 2;
+               ret = VM_FAULT_MAJOR;
        }

        lock_page(page);
@@ -1188,7 +1195,7 @@
                spin_unlock(&mm->page_table_lock);
                unlock_page(page);
                page_cache_release(page);
-               return 1;
+               return VM_FAULT_MINOR;
        }

        /* The page isn't present yet, go ahead with the fault. */
@@ -1246,7 +1253,7 @@
                        pte_unmap(page_table);
                        page_cache_release(page);
                        spin_unlock(&mm->page_table_lock);
-                       return 1;
+                       return VM_FAULT_MINOR;
                }
                mm->rss++;
                flush_page_to_ram(page);
@@ -1260,10 +1267,10 @@
        /* No need to invalidate - it was non-present before */
        update_mmu_cache(vma, addr, entry);
        spin_unlock(&mm->page_table_lock);
-       return 1;       /* Minor fault */
+       return VM_FAULT_MINOR;

 no_mem:
-       return -1;
+       return VM_FAULT_OOM;
 }

 /*
@@ -1291,10 +1298,11 @@

        new_page = vma->vm_ops->nopage(vma, address & PAGE_MASK, 0);

-       if (new_page == NULL)   /* no page was available -- SIGBUS */
-               return 0;
+       /* no page was available -- either SIGBUS or OOM */
+       if (new_page == NOPAGE_SIGBUS)
+               return VM_FAULT_SIGBUS;
        if (new_page == NOPAGE_OOM)
-               return -1;
+               return VM_FAULT_OOM;

        /*
         * Should we do an early C-O-W break?
@@ -1303,7 +1311,7 @@
                struct page * page = alloc_page(GFP_HIGHUSER);
                if (!page) {
                        page_cache_release(new_page);
-                       return -1;
+                       return VM_FAULT_OOM;
                }
                copy_user_highpage(page, new_page, address);
                page_cache_release(new_page);
@@ -1339,13 +1347,13 @@
                pte_unmap(page_table);
                page_cache_release(new_page);
                spin_unlock(&mm->page_table_lock);
-               return 1;
+               return VM_FAULT_MINOR;
        }

        /* no need to invalidate: a not-present page shouldn't be cached */
        update_mmu_cache(vma, address, entry);
        spin_unlock(&mm->page_table_lock);
-       return 2;       /* Major fault */
+       return VM_FAULT_MAJOR;
 }

 /*
@@ -1397,7 +1405,7 @@
        establish_pte(vma, address, pte, entry);
        pte_unmap(pte);
        spin_unlock(&mm->page_table_lock);
-       return 1;
+       return VM_FAULT_MINOR;
 }

 /*
@@ -1425,7 +1433,7 @@
                        return handle_pte_fault(mm, vma, address, write_access, pte, pmd);
        }
        spin_unlock(&mm->page_table_lock);
-       return -1;
+       return VM_FAULT_OOM;
 }

 /*
===== arch/i386/mm/fault.c 1.13 vs edited =====
--- 1.13/arch/i386/mm/fault.c   Tue Feb 26 17:13:20 2002
+++ edited/arch/i386/mm/fault.c Fri Jun  7 15:26:47 2002
@@ -56,12 +56,16 @@

        for (;;) {
        survive:
-               {
-                       int fault = handle_mm_fault(current->mm, vma, start, 1);
-                       if (!fault)
+               switch (handle_mm_fault(current->mm, vma, start, 1)) {
+                       case VM_FAULT_SIGBUS:
                                goto bad_area;
-                       if (fault < 0)
+                       case VM_FAULT_OOM:
                                goto out_of_memory;
+                       case VM_FAULT_MINOR:
+                       case VM_FAULT_MAJOR:
+                               break;
+                       default:
+                               BUG();
                }
                if (!size)
                        break;
@@ -239,16 +243,18 @@
         * the fault.
         */
        switch (handle_mm_fault(mm, vma, address, write)) {
-       case 1:
-               tsk->min_flt++;
-               break;
-       case 2:
-               tsk->maj_flt++;
-               break;
-       case 0:
-               goto do_sigbus;
-       default:
-               goto out_of_memory;
+               case VM_FAULT_MINOR:
+                       tsk->min_flt++;
+                       break;
+               case VM_FAULT_MAJOR:
+                       tsk->maj_flt++;
+                       break;
+               case VM_FAULT_SIGBUS:
+                       goto do_sigbus;
+               case VM_FAULT_OOM:
+                       goto out_of_memory;
+               default:
+                       BUG();
        }

        /*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at   http://www.**-**.com/
Please read the FAQ at   http://www.**-**.com/



 Wed, 24 Nov 2004 08:00:17 GMT   
 remove magic numbers for fault return codes
On Fri, Jun 07, 2002 at 04:52:59PM -0700, William Lee Irwin III wrote:

This is against 2.5.20, sorry for not mentioning that before.

Cheers,
Bill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Wed, 24 Nov 2004 08:00:20 GMT   
 
   [ 2 post ] 

Similar Threads

1. help: fsck returns BAD MAGIC NUMBER

2. Error boot to single-user mode: fsck returns magic number wrong

3. Boot failure returns bad Magic Number

4. remove magic numbers in block queue initialization

5. magic numbers, /etc/magic and the Real World(tm)

6. Help: Returning a return code to a program

7. Help: Returning a return code to a program

8. Return Code or Error Code with patches

9. Returning an exit code with the PID as an exit code

10. help: preserving error return codes thru function codes


 
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by ST Software