RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn

["Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>]

 

>-----Original Message-----
>From: Nick Piggin [mailto:npiggin@suse.de] 
>Sent: Wednesday, November 12, 2008 7:44 PM
>To: Pallipadi, Venkatesh
>Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; 
>Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de 
>Ven; linux-kernel@vger.kernel.org; Siddha, Suresh B
>Subject: Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
>
>On Wed, Nov 12, 2008 at 04:02:47PM -0800, Pallipadi, Venkatesh wrote:
>>
>>
>> >-----Original Message-----
>> >From: Nick Piggin [mailto:npiggin@suse.de]
>> >Sent: Wednesday, November 12, 2008 3:23 PM
>> >To: Pallipadi, Venkatesh
>> >Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins;
>> >Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de
>> >Ven; linux-kernel@vger.kernel.org; Siddha, Suresh B
>> >Subject: Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in 
>vm_insert_pfn
>> >
>> >You have to be careful of this, because it can be called 
>with mmap_sem
>> >held for read only. Hmm, I guess vm_insert_page is doing the
>> >same thing.
>> >Probably mostly works because all other modifiers of vm_flags
>> >are holding
>> >mmap_sem.
>>
>> Yes. I did the patch looking at vm_insert_page doing similar thing.
>>
>> >
>> >However, in some cases, code can do vm_insert_pfn and vm_insert_page
>> >(actually hmm, no vm_insert_mixed actually should cover 
>most of those
>> >cases).
>> >
>> >Still, I'd be much happier if we could make these into 
>BUG_ON, and then
>> >teach callers to set it in their .mmap routines.
>>
>> Actually, vm_insert_pfn() already has a BUG_ON() at the 
>start for cases
>> where neither (or both) MIXEDMAP and PFNMAP is not set. So, 
>that should
>> cover the case we are worried about it here and we can eliminate this
>> patch altogether. Only part I am not sure about is why we are looking
>> for MIXEDMAP here. Shouldn't they be using vm_insert_mixed instead?
>
>They should, but it will do an inesrt_pfn in some cases, won't it?
>

Yes. It does. But, it calls a lower level insert_pfn() function. The lower
level insert_pfn() does not have any bug checks. But the higher level
vm_insert_pfn() checks for PFNMAP or MIXEDMAP.

Thanks,
Venki