questions about ttm_page_alloc.c
in [Kernel]
|
Prev: [PATCH] Staging: vt6656: Remove PUINT from ttype.h
Next: [PATCH] Staging: vt6656: Remove the ULONG_PTR and DWORD_PTR defines from ttype.h
From: Dan Carpenter on 12 Jul 2010 18:50 I'm investigating: https://bugzilla.kernel.org/show_bug.cgi?id=16337 He is using the new radeon with the new ttm pool wc/uc page allocator. I'm sort of over my head when it comes to mm stuff so forgive me if these are dumb questions... I'm looking at drivers/gpu/drm/ttm/ttm_page_alloc.c. 230 static int set_pages_array_wc(struct page **pages, int addrinarray) 231 { 232 #ifdef TTM_HAS_AGP 233 int i; 234 235 for (i = 0; i < addrinarray; i++) 236 map_page_into_agp(pages[i]); ^^^^^^^^^^^^^^^^^^^^^^^^^^^ This actually sets the pages to uncached and not to write cached. Is that deliberate? 237 #endif 238 return 0; 239 } [snip] 327 pages_to_free[freed_pages++] = p; 328 /* We can only remove NUM_PAGES_TO_ALLOC at a time. */ 329 if (freed_pages >= NUM_PAGES_TO_ALLOC) { 330 /* remove range of pages from the pool */ 331 __list_del(p->lru.prev, &pool->list); Why do we use p->lru.prev here when we use &p->lru in other places? 332 333 ttm_pool_update_free_locked(pool, freed_pages); 334 /** 335 * Because changing page caching is costly 336 * we unlock the pool to prevent stalling. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jerome Glisse on 12 Jul 2010 19:20 On 07/12/2010 06:39 PM, Dan Carpenter wrote: > I'm investigating: https://bugzilla.kernel.org/show_bug.cgi?id=16337 > > He is using the new radeon with the new ttm pool wc/uc page allocator. > I'm sort of over my head when it comes to mm stuff so forgive me if > these are dumb questions... I'm looking at > drivers/gpu/drm/ttm/ttm_page_alloc.c. > > 230 static int set_pages_array_wc(struct page **pages, int addrinarray) > 231 { > 232 #ifdef TTM_HAS_AGP > 233 int i; > 234 > 235 for (i = 0; i< addrinarray; i++) > 236 map_page_into_agp(pages[i]); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This actually sets the pages to uncached and not to write > cached. Is that deliberate? > > 237 #endif > 238 return 0; > 239 } > > [snip] > > 327 pages_to_free[freed_pages++] = p; > 328 /* We can only remove NUM_PAGES_TO_ALLOC at a time. */ > 329 if (freed_pages>= NUM_PAGES_TO_ALLOC) { > 330 /* remove range of pages from the pool */ > 331 __list_del(p->lru.prev,&pool->list); > > Why do we use p->lru.prev here when we use&p->lru in other > places? > > 332 > 333 ttm_pool_update_free_locked(pool, freed_pages); > 334 /** > 335 * Because changing page caching is costly > 336 * we unlock the pool to prevent stalling. > > regards, > dan carpenter > _______________________________________________ > dri-devel mailing list > dri-devel(a)lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel No it's fine, this code is for non x86 CPU, and on such platform we assume wc == uncached and wb is normal (ie cached). Cheers, Jerome -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Dan Carpenter on 22 Jul 2010 08:00 On Mon, Jul 12, 2010 at 07:12:37PM -0400, Jerome Glisse wrote: > On 07/12/2010 06:39 PM, Dan Carpenter wrote: >> 327 pages_to_free[freed_pages++] = p; >> 328 /* We can only remove NUM_PAGES_TO_ALLOC at a time. */ >> 329 if (freed_pages>= NUM_PAGES_TO_ALLOC) { >> 330 /* remove range of pages from the pool */ >> 331 __list_del(p->lru.prev,&pool->list); >> >> Why do we use p->lru.prev here when we use &p->lru in other >> places? >> >> 332 >> 333 ttm_pool_update_free_locked(pool, freed_pages); >> 334 /** >> 335 * Because changing page caching is costly >> 336 * we unlock the pool to prevent stalling. >> Thanks for answering about the wb vs uncached, but I'm still confused why we use &p->lru in most places and p->lru.prev in this place. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jerome Glisse on 22 Jul 2010 10:20
On 07/22/2010 07:56 AM, Dan Carpenter wrote: > On Mon, Jul 12, 2010 at 07:12:37PM -0400, Jerome Glisse wrote: >> On 07/12/2010 06:39 PM, Dan Carpenter wrote: >>> 327 pages_to_free[freed_pages++] = p; >>> 328 /* We can only remove NUM_PAGES_TO_ALLOC at a time. */ >>> 329 if (freed_pages>= NUM_PAGES_TO_ALLOC) { >>> 330 /* remove range of pages from the pool */ >>> 331 __list_del(p->lru.prev,&pool->list); >>> >>> Why do we use p->lru.prev here when we use&p->lru in other >>> places? >>> >>> 332 >>> 333 ttm_pool_update_free_locked(pool, freed_pages); >>> 334 /** >>> 335 * Because changing page caching is costly >>> 336 * we unlock the pool to prevent stalling. >>> > > Thanks for answering about the wb vs uncached, but I'm still confused why we use > &p->lru in most places and p->lru.prev in this place. > > regards, > dan carpenter > This is because it use __list_del to remove a whole part of the list. /* * Delete a list entry by making the prev/next entries * point to each other. * * This is only for internal list manipulation where we know * the prev/next entries already! */ static inline void __list_del(struct list_head * prev, struct list_head * next) { ��������next->prev = prev; ��������prev->next = next; } Cheers, Jerome -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |