lkml.org 
[lkml]   [2013]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 10/15] KVM: MMU: allocate shadow pages from slab
    On Thu, Oct 24, 2013 at 07:01:49PM +0800, Xiao Guangrong wrote:
    > On 10/24/2013 06:39 PM, Gleb Natapov wrote:
    > > On Thu, Oct 24, 2013 at 06:10:46PM +0800, Xiao Guangrong wrote:
    > >> On 10/24/2013 05:52 PM, Gleb Natapov wrote:
    > >>> On Thu, Oct 24, 2013 at 05:29:44PM +0800, Xiao Guangrong wrote:
    > >>>> On 10/24/2013 05:19 PM, Gleb Natapov wrote:
    > >>>>
    > >>>>>> @@ -946,7 +947,7 @@ static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
    > >>>>>> {
    > >>>>>> struct page *page = pfn_to_page(shadow_page >> PAGE_SHIFT);
    > >>>>>>
    > >>>>>> - return (struct kvm_mmu_page *)page_private(page);
    > >>>>>> + return (struct kvm_mmu_page *)(page->mapping);
    > >>>>> Why?
    > >>>>
    > >>>> That's because page->private has been used by slab:
    > >>>>
    > >>> But does lockless path actually looks at it?
    > >>
    > >> Lockless path does not use it, however, it is used by kvm_mmu_page():
    > >>
    > >> static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
    > >> {
    > >> struct page *page = pfn_to_page(shadow_page >> PAGE_SHIFT);
    > >>
    > >> return (struct kvm_mmu_page *)(page->mapping);
    > >> }
    > >>
    > >> which is used in the common code.
    > > Ah, so the pointer is not available even after object is allocated.
    > > Make sense since we allocate object, not page here, but is it safe to
    > > use mapping like that?
    >
    > The commens says:
    >
    > struct address_space *mapping; /* If low bit clear, points to
    > * inode address_space, or NULL.
    > * If page mapped as anonymous
    > * memory, low bit is set, and
    > * it points to anon_vma object:
    > * see PAGE_MAPPING_ANON below.
    >
    > It seems mapping is used for address_space or anonymous memory, in
    > our case, the page is used by slab, so I guess it is ok. And the bug
    > i put in set_page_header() was not tiggered on both slab and slub.
    >
    Yeah, I also think so. I asked Andrea (copied) and he thinks that it is
    safe too currently, but things changes fast in this area. Andrea?
    Another option is too save slab_cache pointer and reset it before
    freeing the object but it looks ugly.

    --
    Gleb.


    \
     
     \ /
      Last update: 2013-10-24 14:41    [W:6.387 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site