<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body>
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>
https://jira.whamcloud.com/browse/LU-11403<br>
<br>
Which is when the file exists but has no striping info, and hence no data.<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> Tuesday, May 21, 2019 10:54:37 PM<br>
<b>To:</b> James Simmons; Andreas Dilger; Oleg Drokin<br>
<b>Cc:</b> Lustre Development List; Patrick Farrell; James Simmons<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 Mon, May 20 2019, James Simmons wrote:<br>
<br>
> From: Patrick Farrell <pfarrell@whamcloud.com><br>
><br>
> Various error conditions in the fault path can cause us to<br>
> not return a page in vm_fault.  Check if it's present<br>
> before accessing it.<br>
<br>
I cannot find any error conditions that would leave ->page NULL,<br>
but that wouldn't set one of<br>
  VM_FAULT_RETRY | VM_FAULT_ERROR | VM_FAULT_LOCKED<br>
in 'result'.<br>
<br>
Can someone provide an example?<br>
<br>
(I have seen crashes with vmf->page being NULL, but they were caused<br>
 by VM_FAULT_RETRY being #defined to 0 as lustre/llite/llite_internal.h<br>
 still does on OpenSFS lustre)<br>
<br>
><br>
> Additionally, it's not valid to return VM_FAULT_NOPAGE for<br>
> page faults.  The correct return when accessing a page that<br>
> does not exist is VM_FAULT_SIGBUS.  Correcting this avoids<br>
> looping infinitely in the testcase.<br>
<br>
I agree with that.  VM_FAULT_NOPAGE is valid for page_mkwrite - and<br>
ll_page_mkwrite() has separate code to choose VM_FAULT_NOPAGE.<br>
So the change to to_fault_error() is valid.<br>
<br>
NeilBrown<br>
<br>
<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>
> ---<br>
>  fs/lustre/llite/llite_mmap.c | 6 ++----<br>
>  1 file changed, 2 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/fs/lustre/llite/llite_mmap.c b/fs/lustre/llite/llite_mmap.c<br>
> index 1865db1..c8e57ad 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>
> @@ -366,7 +363,8 @@ static vm_fault_t ll_fault(struct vm_fault *vmf)<br>
>  <br>
>  restart:<br>
>        result = __ll_fault(vmf->vma, vmf);<br>
> -     if (!(result & (VM_FAULT_RETRY | VM_FAULT_ERROR | VM_FAULT_LOCKED))) {<br>
> +     if (vmf->page &&<br>
> +         !(result & (VM_FAULT_RETRY | VM_FAULT_ERROR | VM_FAULT_LOCKED))) {<br>
>                struct page *vmpage = vmf->page;<br>
>  <br>
>                /* check if this page has been truncated */<br>
> -- <br>
> 1.8.3.1<br>
</div>
</span></font></div>
</body>
</html>