From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Souptick Joarder <jrdr.linux@gmail.com>
Cc: rdunlap@infradead.org, willy@infradead.org, linux-nvdimm@lists.01.org
Subject: Re: [PATCH v2] dax: Change return type to vm_fault_t
Date: Mon, 16 Apr 2018 17:02:00 -0600 [thread overview]
Message-ID: <20180416230200.GB24742@linux.intel.com> (raw)
In-Reply-To: <20180416185044.GA29337@jordon-HP-15-Notebook-PC>
On Tue, Apr 17, 2018 at 12:20:44AM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault and huge_fault
> handler. For now, this is just documenting that the
> function returns a VM_FAULT value rather than an errno.
> Once all instances are converted, vm_fault_t will become
> a distinct type.
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,
};
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?
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
>
> Previously vm_insert_mixed() returns err which driver
> mapped into VM_FAULT_* type. The new function
> vmf_insert_mixed() will replace this inefficiency by
> returning VM_FAULT_* type.
>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> Reviewed-by: Matthew Wilcox <mawilcox@microsoft.com>
> ---
> v2: Modified the change log
>
> v3: Updated the change log
You've only got a v2. :)
Sure, the code looks correct. You can add:
Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
next prev parent reply other threads:[~2018-04-16 23:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-16 18:50 [PATCH v2] dax: Change return type to vm_fault_t Souptick Joarder
2018-04-16 23:02 ` Ross Zwisler [this message]
2018-04-16 23:21 ` Matthew Wilcox
2018-04-17 5:21 ` Souptick Joarder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180416230200.GB24742@linux.intel.com \
--to=ross.zwisler@linux.intel.com \
--cc=jrdr.linux@gmail.com \
--cc=linux-nvdimm@lists.01.org \
--cc=rdunlap@infradead.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).