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 [off-list ref] wrote:

quoted

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.