Messages in this thread | | | Date | Wed, 3 Aug 2005 12:47:14 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: [patch 2.6.13-rc4] fix get_user_pages bug |
| |
On Wed, 3 Aug 2005, Nick Piggin wrote: > Hugh Dickins wrote: > > > > Here we are: get_user_pages quite untested, let alone the racy case, > > but I think it should work. Please all hack it around as you see fit, > > I'll check mail when I get home, but won't be very responsive... > > Seems OK to me. I don't know why you think handle_mm_fault can't > be inline, but if it can be, then I have a modification attached > that removes the condition - any good?
Stupidity was the reason I thought handle_mm_fault couldn't be inline: I was picturing it static inline within mm/memory.c, failed to make the great intellectual leap you've achieved by moving it to include/linux/mm.h.
> Oh, it gets rid of the -1 for VM_FAULT_OOM. Doesn't seem like there > is a good reason for it, but might that break out of tree drivers?
No, I don't think it would break anything: it's just an historic oddity, used to be -1 for failure, and only got given a name recently, I think when wli added the proper major/minor counting.
Your version of the patch looks less hacky to me (not requiring VM_FAULT_WRITE_EXPECTED arg), though we could perfectly well remove that at leisure by adding VM_FAULT_WRITE case into all the arches in 2.6.14 (which might be preferable to leaving the __inline obscurity?).
I don't mind either way, but since you've not yet found an actual error in mine, I'd prefer you to make yours a tidyup patch on top, Signed-off-by your own good self, and let Linus decide whether he wants to apply yours on top or not. Or perhaps the decision rests for the moment with Robin, whether he gets his customer to test yours or mine - whichever is tested is the one which should go in.
Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |