On Mon, 1 Aug 2005, Nick Piggin wrote: > > This was tested by Robin and appears to solve the problem. Roland > had a quick look and thought the basic idea was sound. I'd like to > get a couple more acks before going forward, and in particular > Robin was contemplating possible efficiency improvements (although > efficiency can wait on correctness).
I'd much prefer a solution that doesn't invade all the arches, but I don't think Linus' pte_dirty method will work on s390 (unless we change s390 in a less obvious way than your patch), so it looks like we do need something like yours. Comments: There are currently 21 architectures, but so far your patch only updates 14 of them? Personally (rather like Robin) I'd have preferred a return code more directed to the issue at hand, than your VM_FAULT_RACE. Not for efficiency reasons, I think you're right that's not a real issue, but more to document the peculiar case we're addressing. Perhaps a code that says do_wp_page has gone all the way through, even though it hasn't set the writable bit. That would require less change in mm/memory.c, but I've no strong reason to argue that you change your approach in that way. Perhaps others prefer the race case singled out, to gather statistics on that. Assuming we stick with your VM_FAULT_RACE... Could we define VM_FAULT_RACE as 3 rather than -2? I think there's some historical reason why VM_FAULT_OOM is -1, but see no cause to extend the range in that strange direction. VM_FAULT_RACE is a particular subcase of VM_FAULT_MINOR: throughout the arches I think they should just be adjacent cases of the switch statement, both doing the min_flt++. Your continue in get_user_pages skips page_table_lock as Linus noted. The do_wp_page call from do_swap_page needs to be adjusted, to return VM_FAULT_RACE if that's what it returned. If VM_FAULT_RACE is really recording races, then the bottom return value from handle_pte_fault ought to be VM_FAULT_RACE rather than VM_FAULT_MINOR. Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/