<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body>
Neil,<br>
<br>
Memory has surfaced here. There was a not-documented-where-I-can-find-it report of a null pointer on “page” here that came in around the same time running - I believe - racer (a big messy “do random things from many threads” test). No obvious explanation
for how we got the problem but I added the check since it was simple enough.<br>
<br>
But I’m perfectly comfortable with you removing it and seeing if something occurs again.<br>
<br>
- Patrick
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> NeilBrown <neilb@suse.com><br>
<b>Sent:</b> Wednesday, May 22, 2019 6:26:47 PM<br>
<b>To:</b> Patrick Farrell; James Simmons; Andreas Dilger; Oleg Drokin<br>
<b>Cc:</b> Lustre Development List<br>
<b>Subject:</b> Re: [PATCH v2 01/29] lustre: llite: ll_fault fixes</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Wed, May 22 2019, Patrick Farrell wrote:<br>
<br>
> Neil,<br>
><br>
> If you can’t find any, I imagine they don’t exist, at least in your branch, given that difference you cited.<br>
><br>
> The particular case we had is here:<br>
> <a href="https://jira.whamcloud.com/browse/LU-11403">https://jira.whamcloud.com/browse/LU-11403</a><br>
><br>
> Which is when the file exists but has no striping info, and hence no data.<br>
<br>
Hi Patrick,<br>
Thanks for the reply. Looking at that issue it appears that the root<br>
cause is -EFAULT being returned from lov_io_init_empty, which then got<br>
returned by ll_fault_io_init and then was converted by to_fault_error<br>
into VM_FAULT_NOPAGE.<br>
<br>
The test<br>
<br>
if (!(result & (VM_FAULT_RETRY | VM_FAULT_ERROR | VM_FAULT_LOCKED))) {<br>
<br>
in ll_fault() doesn't notice VM_FAULT_NOPAGE (it is not part of<br>
VM_FAULT_ERROR), so the 'then' branch is run, which triggers the<br>
problem.<br>
<br>
Just changing to_fault_error() to convert -EFAULT to VM_FAULT_SIGBUS,<br>
as you did, cause the 'if' to do "the right thing", as VM_FAULT_SIGBUS<br>
is one of the flags in VM_FAULT_ERROR.<br>
<br>
So the extra test on vmf->page is redundant. If there has been no<br>
error, and we don't need to retry, then vmf->page *must* exist.<br>
<br>
<br>
I've changed the commit message to explain this, so what I have now is<br>
below.<br>
<br>
Thanks,<br>
NeilBrown<br>
<br>
From: Patrick Farrell <pfarrell@whamcloud.com><br>
Date: Mon, 20 May 2019 08:50:43 -0400<br>
Subject: [PATCH] lustre: llite: ll_fault fix<br>
<br>
Error conditions in the fault path such as a fault on a file without<br>
stripes (see lov_io_init_emtpy()) can cause us to<br>
not return a page in vm_fault, and to report -EFAULT.<br>
<br>
-EFAULT is currently mapped to VM_FAULT_NOPAGE by to_fault_error(),<br>
and ll_fault doesn't recognize this as an error which might leave<br>
->page unset, and tries to dereference vmf->page.<br>
<br>
VM_FAULT_NOPAGE is *not* a valid status for .fault, only for<br>
.page_mkwrite and .pfn_mkwrite. So change to_fault_error() to<br>
instead map -EFAULT to VM_FAULT_SIGBUS. This is part of<br>
VM_FAULT_ERROR, so ll_fault will *not* dereference vmf->page when that<br>
error code is set.<br>
<br>
Signed-off-by: Patrick Farrell <pfarrell@whamcloud.com><br>
WC-bug-id: <a href="https://jira.whamcloud.com/browse/LU-11403">https://jira.whamcloud.com/browse/LU-11403</a><br>
Reviewed-on: <a href="https://review.whamcloud.com/34247">https://review.whamcloud.com/34247</a><br>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com><br>
Reviewed-by: Alexander Zarochentsev <c17826@cray.com><br>
Reviewed-by: Oleg Drokin <green@whamcloud.com><br>
Signed-off-by: James Simmons <jsimmons@infradead.org><br>
Signed-off-by: NeilBrown <neilb@suse.com><br>
---<br>
fs/lustre/llite/llite_mmap.c | 3 ---<br>
1 file changed, 3 deletions(-)<br>
<br>
diff --git a/fs/lustre/llite/llite_mmap.c b/fs/lustre/llite/llite_mmap.c<br>
index 1865db1237ce..59fb40029306 100644<br>
--- a/fs/lustre/llite/llite_mmap.c<br>
+++ b/fs/lustre/llite/llite_mmap.c<br>
@@ -238,9 +238,6 @@ static inline vm_fault_t to_fault_error(int result)<br>
case 0:<br>
result = VM_FAULT_LOCKED;<br>
break;<br>
- case -EFAULT:<br>
- result = VM_FAULT_NOPAGE;<br>
- break;<br>
case -ENOMEM:<br>
result = VM_FAULT_OOM;<br>
break;<br>
-- <br>
2.14.0.rc0.dirty<br>
<br>
</div>
</span></font></div>
</body>
</html>