On Mon, Apr 16, 2018 at 05:02:00PM -0600, Ross Zwisler wrote:
> I'm not sure what you mean by "vm_fault_t will become a distinct type"?  Do
> you mean you'll make it into an enum, i.e.: 
> 
> enum vm_fault_t {
>       VM_FAULT_OOM            = 0x0001,
>       VM_FAULT_SIGBUS         = 0x0002,
>       VM_FAULT_MAJOR          = 0x0004,
>       VM_FAULT_WRITE          = 0x0008,
>       VM_FAULT_HWPOISON       = 0x0010,
>       VM_FAULT_HWPOISON_LARGE = 0x0020,
>       VM_FAULT_NOPAGE         = 0x0100,
>       VM_FAULT_LOCKED         = 0x0200,
>       VM_FAULT_RETRY          = 0x0400,
>       VM_FAULT_FALLBACK       = 0x0800,
>       VM_FAULT_DONE_COW       = 0x1000,
>       VM_FAULT_NEEDDSYNC      = 0x2000,
> };

My current tree has this in it:

+/*
+ * Different kinds of faults, as returned from fault handlers.
+ * Used to decide whether a process gets delivered SIGBUS or
+ * just gets major/minor fault counters bumped up.
+ */
+typedef __bitwise unsigned int vm_fault_t;
+enum {
+       VM_FAULT_OOM            = (__force vm_fault_t)0x000001, /* Out Of 
Memory */
+       VM_FAULT_SIGBUS         = (__force vm_fault_t)0x000002, /* Bad access */
+       VM_FAULT_MAJOR          = (__force vm_fault_t)0x000004, /* Page read 
from storage */
+       VM_FAULT_WRITE          = (__force vm_fault_t)0x000008, /* Special case 
for get_user_pages */
+       VM_FAULT_HWPOISON       = (__force vm_fault_t)0x000010, /* Hit poisoned 
small page */
+       VM_FAULT_HWPOISON_LARGE = (__force vm_fault_t)0x000020, /* Hit poisoned 
large page. Index encoded in upper bits */
+       VM_FAULT_SIGSEGV        = (__force vm_fault_t)0x000040,
+       VM_FAULT_NOPAGE         = (__force vm_fault_t)0x000100, /* ->fault 
installed the pte, not return page */
+       VM_FAULT_LOCKED         = (__force vm_fault_t)0x000200, /* ->fault 
locked the returned page */
+       VM_FAULT_RETRY          = (__force vm_fault_t)0x000400, /* ->fault 
blocked, must retry */
+       VM_FAULT_FALLBACK       = (__force vm_fault_t)0x000800, /* huge page 
fault failed, fall back to small */
+       VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000, /* ->fault has 
fully handled COW */
+       VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000, /* ->fault did 
not modify page tables
+                                        * and needs fsync() to complete (for
+                                        * synchronous page faults in DAX) */
+       VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
+};

However, I'm hoping to get a better syntax into sparse ;-)

> If so, I think that would be great for readability.  Right now I look up the
> definition of vm_fault_t and see it's typedef'd to an int, and that doesn't
> give me as much info as the above enum would.
> 
> If this is the plan, I don't understand why you need to make all the site
> conversions first?

Changing it from a signed to an unsigned int causes GCC to warn about
an assignment from an incompatible type -- int foo(void) is incompatible
with unsigned int foo(void).

> Sure, the code looks correct. You can add:
> Reviewed-by: Ross Zwisler <ross.zwis...@linux.intel.com>

Thanks, Ross.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to