On Fri, May 04, 2018 at 11:10:54AM +0200, Laurent Dufour wrote:

On 03/05/2018 17:42, Minchan Kim wrote:

quoted

On Thu, May 03, 2018 at 02:25:18PM +0200, Laurent Dufour wrote:

quoted

On 23/04/2018 09:42, Minchan Kim wrote:

quoted

On Tue, Apr 17, 2018 at 04:33:18PM +0200, Laurent Dufour wrote:

quoted

When handling speculative page fault, the vma->vm_flags and
vma->vm_page_prot fields are read once the page table lock is released. So
there is no more guarantee that these fields would not change in our back
They will be saved in the vm_fault structure before the VMA is checked for
changes.

Sorry. I cannot understand.
If it is changed under us, what happens? If it's critical, why cannot we
check with seqcounter?
Clearly, I'm not understanding the logic here. However, it's a global
change without CONFIG_SPF so I want to be more careful.
It would be better to describe why we need to sanpshot those values
into vm_fault rather than preventing the race.

The idea is to go forward processing the page fault using the VMA's fields
values saved in the vm_fault structure. Then once the pte are locked, the
vma->sequence_counter is checked again and if something has changed in our back
the speculative page fault processing is aborted.

Sorry, still I don't understand why we should capture some fields to vm_fault.
If we found vma->seq_cnt is changed under pte lock, can't we just bail out and
fallback to classic fault handling?

Maybe, I'm missing something clear now. It would be really helpful to understand
if you give some exmaple.

I'd rather say that I was not clear enough ;)

Here is the point, when we deal with a speculative page fault, the mmap_sem is
not taken, so parallel VMA's changes can occurred. When a VMA change is done
which will impact the page fault processing, we assumed that the VMA sequence
counter will be changed.

In the page fault processing, at the time the PTE is locked, we checked the VMA
sequence counter to detect changes done in our back. If no change is detected
we can continue further. But this doesn't prevent the VMA to not be changed in
our back while the PTE is locked. So VMA's fields which are used while the PTE
is locked must be saved to ensure that we are using *static* values.
This is important since the PTE changes will be made on regards to these VMA
fields and they need to be consistent. This concerns the vma->vm_flags and
vma->vm_page_prot VMA fields.

I hope I make this clear enough this time.

It's more clear at this point. Please include such nice explanation in description.
Now, I am wondering how you synchronize those static value and vma's seqcount.
It must be in next patchset. I hope to grab a time to read it, asap.

Thanks.