Hi Andrew On Fri, Jan 24, 2014 at 2:49 AM, Andrew Morton <[email protected]> wrote: > On Thu, 23 Jan 2014 20:27:29 +0400 (MSK) malc <[email protected]> wrote: > >> Sep 17 00:00:00 2001 >> From: Vladimir Murzin <[email protected]> >> Date: Thu, 23 Jan 2014 14:54:20 +0400 >> Subject: [PATCH] Revert "mm/vmalloc: interchage the implementation of >> vmalloc_to_{pfn,page}" >> >> This reverts commit ece86e222db48d04bda218a2be70e384518bb08c. >> >> Despite being claimed that patch doesn't introduce any functional >> changes in fact it does. >> >> The "no page" path behaves different now. Originally, vmalloc_to_page >> might return NULL under some conditions, with new implementation it returns >> pfn_to_page(0) which is not the same as NULL. >> >> Simple test shows the difference. >> >> test.c >> >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/vmalloc.h> >> #include <linux/mm.h> >> >> int __init myi(void) >> { >> struct page *p; >> void *v; >> >> v = vmalloc(PAGE_SIZE); >> /* trigger the "no page" path in vmalloc_to_page*/ >> vfree(v); >> >> p = vmalloc_to_page(v); >> >> pr_err("expected val = NULL, returned val = %p", p); >> >> return -EBUSY; >> } >> >> void __exit mye(void) >> { >> >> } >> module_init(myi) >> module_exit(mye) >> >> Before interchange: >> expected val = NULL, returned val = (null) >> >> After interchange: >> expected val = NULL, returned val = c7ebe000 >> > > hm, yes, I suppose that's bad. > > Rather than reverting the patch we could fix up vmalloc_to_pfn() and/or > vmalloc_to_page() to handle this situation. Did you try that? >
Personally, I didn't try; I leaved this responsibility to the author of the patch as a review feedback. Unfortunately, there was no any response. Being said that original patch makes vmalloc_to_* "slightly more efficient", I'm in doubt that with additional handling it'd still improve something. I'd be very glad if someone point me at the benefit of the patch - just to have an idea why we need to put extra effort here. Thanks Vladimir > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to [email protected]. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"[email protected]";> [email protected] </a> -- 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/
