2012/5/19 Christoph Lameter <cl@xxxxxxxxx>:
> -/*
> * free_slob_page: call before a slob_page is returned to the page allocator.
> */
> -static inline void free_slob_page(struct slob_page *sp)
> +static inline void free_slob_page(struct page *sp)
> {
> - reset_page_mapcount(&sp->page);
> - sp->page.mapping = NULL;
> + reset_page_mapcount(sp);
> + sp->mapping = NULL;
> }
Currently, sp->mapping = NULL is useless, because Slob doesn't touch
this field anymore.
> /*
> @@ -133,44 +112,44 @@ static LIST_HEAD(free_slob_large);
> /*
> * is_slob_page: True for all slob pages (false for bigblock pages)
> */
> -static inline int is_slob_page(struct slob_page *sp)
> +static inline int is_slob_page(struct page *sp)
> {
> - return PageSlab((struct page *)sp);
> + return PageSlab(sp);
> }
>
> -static inline void set_slob_page(struct slob_page *sp)
> +static inline void set_slob_page(struct page *sp)
> {
> - __SetPageSlab((struct page *)sp);
> + __SetPageSlab(sp);
> }
>
> -static inline void clear_slob_page(struct slob_page *sp)
> +static inline void clear_slob_page(struct page *sp)
> {
> - __ClearPageSlab((struct page *)sp);
> + __ClearPageSlab(sp);
> }
Now, type casting is useless, so using __SetPageSlab() is possible.
If we use __SetPageSlab() directly, we lose some readability.
Which one is preferable?
> -static inline struct slob_page *slob_page(const void *addr)
> +static inline struct page *slob_page(const void *addr)
> {
> - return (struct slob_page *)virt_to_page(addr);
> + return virt_to_page(addr);
> }
It is redundant, just using virt_to_page(addr) directly is more preferable
> /*
> * slob_page_free: true for pages on free_slob_pages list.
> */
> -static inline int slob_page_free(struct slob_page *sp)
> +static inline int slob_page_free(struct page *sp)
> {
> - return PageSlobFree((struct page *)sp);
> + return PageSlobFree(sp);
> }
>
> -static void set_slob_page_free(struct slob_page *sp, struct list_head *list)
> +static void set_slob_page_free(struct page *sp, struct list_head *list)
> {
> list_add(&sp->list, list);
> - __SetPageSlobFree((struct page *)sp);
> + __SetPageSlobFree(sp);
> }
>
> -static inline void clear_slob_page_free(struct slob_page *sp)
> +static inline void clear_slob_page_free(struct page *sp)
> {
> list_del(&sp->list);
> - __ClearPageSlobFree((struct page *)sp);
> + __ClearPageSlobFree(sp);
> }
I think we shouldn't use __ClearPageSlobFree anymore.
Before this patch, list_del affect page->private,
so when we manipulate slob list,
using PageSlobFree overloaded with PagePrivate is reasonable.
But, after this patch is applied, list_del doesn't touch page->private,
so manipulate PageSlobFree is not reasonable.
We would use another method for checking slob_page_free without
PageSlobFree flag.
> Index: linux-2.6/include/linux/mm_types.h
> ===================================================================
> --- linux-2.6.orig/include/linux/mm_types.h 2012-05-16 04:38:50.131864458 -0500
> +++ linux-2.6/include/linux/mm_types.h 2012-05-17 03:28:03.630162187 -0500
> @@ -52,7 +52,7 @@ struct page {
> struct {
> union {
> pgoff_t index; /* Our offset within mapping. */
> - void *freelist; /* slub first free object */
> + void *freelist; /* slub/slob first free object */
> };
>
> union {
> @@ -80,11 +80,12 @@ struct page {
> */
> atomic_t _mapcount;
>
> - struct {
> + struct { /* SLUB */
> unsigned inuse:16;
> unsigned objects:15;
> unsigned frozen:1;
> };
> + int units; /* SLOB */
> };
> atomic_t _count; /* Usage count, see below. */
> };
> @@ -96,6 +97,7 @@ struct page {
> struct list_head lru; /* Pageout list, eg. active_list
> * protected by zone->lru_lock !
> */
> + struct list_head list; /* slobs list of pages */
> struct { /* slub per cpu partial pages */
> struct page *next; /* Next partial slab */
> #ifdef CONFIG_64BIT
>
When we define field in mm_types.h for slauob,
sorted order between these is good for readability.
For example, in case of lru, list for slob is first,
but in case of _mapcount, field for slub is first.
Consistent ordering is more preferable I think.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href