Subject: Re: [PATCH] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix

From: Austin S Hemmelgarn <[email-masked]>
Date: Fri, 17 Jul 2015 15:09:15 -0400

On 2015-07-17 14:54, Oleg Nesterov wrote:
> On 07/17, Austin S Hemmelgarn wrote:
>>
>> On 2015-07-17 14:19, Oleg Nesterov wrote:
>>> On 07/17, Austin S Hemmelgarn wrote:
>>>>
>>>> On 2015-07-17 13:55, Oleg Nesterov wrote:
>>>>> On 07/17, Benjamin LaHaise wrote:
>>>>>>
>>>>>> Don't add BUG().  It's the equivalent approach of saying "I think this code
>>>>>> isn't needed, but I'm lazy and not going to remove it properly."
>>>>>
>>>>> There is another interpretation: I think this code must be never called,
>>>>> if it is actually called we have a serious problem which should be loudly
>>>>> reported.
>>>>>
>>>> And not compiling it at all _will_ loudly report it, it'll just report
>>>> it during linking instead of at run-time, which is a much better time to
>>>> shout about it.
>>>
>>> And how can we do this?
>>>
>> If a function that isn't defined (for example, you use a #if block to
>> comment it out under certain circumstances), then the link will fail
>> rather noisily something references it.
>
> This is what we are trying to fix.
Ah, I misunderstood the intent of the patch, looking at it again this 
makes perfect sense.
>
>> We already know during the
>> compile that it's a NOMMU kernel, so anything that calls it on a MMU
>> enabled kernel can have a compile time check added
>
> It already has. memory.c is not compiled if NOMMU.
>
> The problem is aio_ring_vm_ops which references this function. And btw
> filemap_fault() too.
>
> And just in case, I won't mind to add ifdef(CONFIG_MMU) there, I am
> waiting for reply from Benjamin.
>
>> instead of doing the
>> check at runtime (or even just calling it without checking), thus even
>> further reducing code size.
>
> So what exactly do you suggest to fix the problem?
Some sort of COMPILE_BUG_ON usage (I think that's the name of the macro 
that I'm thinking of, not certain though) possibly?  My only point is 
that it's more useful if the function should never be called to find a 
way to enforce this at build time instead of runtime (even more so for 
NOMMU systems, they tend (in my experience at least) to be even more of 
a pain to debug when they crash).  I do agree though that a helpful 
message is much preferred to a link error.