[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <52F6972C.2030307@gentoo.org>
Date: Sat, 08 Feb 2014 15:44:28 -0500
From: Richard Yao <ryao@...too.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
CC: Mel Gorman <mgorman@...e.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Rik van Riel <riel@...hat.com>,
Eric Van Hensbergen <ericvh@...il.com>,
Ron Minnich <rminnich@...dia.gov>,
Latchesar Ionkov <lucho@...kov.net>,
"David S. Miller" <davem@...emloft.net>,
V9FS Develooper Mailing List
<v9fs-developer@...ts.sourceforge.net>,
Linux Netdev Mailing List <netdev@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
Will Deacon <will.deacon@....com>,
Christopher Covington <cov@...eaurora.org>,
Matthew Thode <mthode@...ode.org>
Subject: Re: [PATCH 1/2] mm/vmalloc: export is_vmalloc_or_module_addr
On 02/08/2014 03:06 PM, Linus Torvalds wrote:
> On Sat, Feb 8, 2014 at 11:58 AM, Richard Yao <ryao@...too.org> wrote:
>>
>> My apologies for that. Here is the backtrace:
>>
>> [<ffffffff814878ce>] p9_virtio_zc_request+0x45e/0x510
>> [<ffffffff814814ed>] p9_client_zc_rpc.constprop.16+0xfd/0x4f0
>> [<ffffffff814839dd>] p9_client_read+0x15d/0x240
>> [<ffffffff811c8440>] v9fs_fid_readn+0x50/0xa0
>> [<ffffffff811c84a0>] v9fs_file_readn+0x10/0x20
>> [<ffffffff811c84e7>] v9fs_file_read+0x37/0x70
>> [<ffffffff8114e3fb>] vfs_read+0x9b/0x160
>> [<ffffffff81153571>] kernel_read+0x41/0x60
>> [<ffffffff810c83ab>] copy_module_from_fd.isra.34+0xfb/0x180
>
> So copy_module_from_fd() does read into a vmalloc'ed buffer (which
> isn't pretty, but at least it's not like using some statically
> allocated module data), but that's a regular vmalloc() afaik.
>
> So I don't see the need for "is_vmalloc_or_module_addr()". The regular
> "is_vmalloc_addr()" should be fine, and *is* usable from modules (it's
> inline, not exported, but it comes to the same thing wrt module use),
> exactly because we have traditionally allowed vmalloc'ed memory to be
> used.
>
> So is there some reason why it's not just using that simpler function?
My first instinct was to use "is_vmalloc_addr()" as you suggest.
However, is_vmalloc_addr() only applies to the vmalloc region. While all
architectures load kernel modules into virtual memory (to my knowledge),
some architectures do not load them into the vmalloc region.
is_vmalloc_or_module_addr() contains a comment that clearly states this:
int is_vmalloc_or_module_addr(const void *x)
{
/*
* ARM, x86-64 and sparc64 put modules in a special place,
* and fall back on vmalloc() if that fails. Others
* just put it in the vmalloc space.
*/
#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
unsigned long addr = (unsigned long)x;
if (addr >= MODULES_VADDR && addr < MODULES_END)
return 1;
#endif
return is_vmalloc_addr(x);
}
My inspection of the actual code agreed with that comment (at least for
S390), so I decided against using "is_vmalloc_addr()" here.
Download attachment "signature.asc" of type "application/pgp-signature" (902 bytes)
Powered by blists - more mailing lists