Messages in this thread | | | From | "Pallipadi, Venkatesh" <> | Date | Thu, 13 Nov 2008 10:47:23 -0800 | Subject | RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn |
| |
>-----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
| |