On Tue, 22 May 2012, JoonSoo Kim wrote:

> 2012/5/19 Christoph Lameter <cl@linux.com>:
>
> > -/*
> >  * 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.

Ok. Adding another patch that does this.

> It is redundant, just using virt_to_page(addr) directly is more preferable

Ok adding another patch that avoids the accessors.

> > +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.

What method should we be using?

> 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.

Ok. Reordered for next patchset (probably Friday).