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

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Nick Piggin [mailto:npiggin@xxxxxxx] 
>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@xxxxxxxxxxxxxxx; 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?

Thanks,
Venki

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/