Hi Hui,
On Thu, Jul 20, 2017 at 05:33:45PM +0800, Hui Zhu wrote:
< snip >
> >> >> +++ b/mm/zsmalloc.c
> >> >> @@ -1982,6 +1982,7 @@ int zs_page_migrate(struct address_space
> >> >> *mapping, struct page *newpage,
> >> >> unsigned long old_obj, new_obj;
> >> >> unsigned int obj_idx;
> >> >> int ret = -EAGAIN;
> >> >> + int inuse;
> >> >>
> >> >> VM_BUG_ON_PAGE(!PageMovable(page), page);
> >> >> VM_BUG_ON_PAGE(!PageIsolated(page), page);
> >> >> @@ -1996,21 +1997,24 @@ int zs_page_migrate(struct address_space
> >> >> *mapping, struct page *newpage,
> >> >> offset = get_first_obj_offset(page);
> >> >>
> >> >> spin_lock(&class->lock);
> >> >> - if (!get_zspage_inuse(zspage)) {
> >> >> + inuse = get_zspage_inuse(zspage);
> >> >> + if (mode == MIGRATE_ASYNC && !inuse) {
> >> >> ret = -EBUSY;
> >> >> goto unlock_class;
> >> >> }
> >> >>
> >> >> pos = offset;
> >> >> s_addr = kmap_atomic(page);
> >> >> - while (pos < PAGE_SIZE) {
> >> >> - head = obj_to_head(page, s_addr + pos);
> >> >> - if (head & OBJ_ALLOCATED_TAG) {
> >> >> - handle = head & ~OBJ_ALLOCATED_TAG;
> >> >> - if (!trypin_tag(handle))
> >> >> - goto unpin_objects;
> >> >> + if (inuse) {
> >
> > I don't want to add inuse check for every loop. It might avoid unncessary
> > looping in every loop of zs_page_migrate so it is for optimization, not
> > correction. As I consider it would happen rarely, I think we don't need
> > to add the check. Could you just remove get_zspage_inuse check, instead?
> >
> > like this.
> >
> >
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 013eea76685e..2d3d75fb0f16 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -1980,14 +1980,9 @@ int zs_page_migrate(struct address_space *mapping,
> > struct page *newpage,
> > pool = mapping->private_data;
> > class = pool->size_class[class_idx];
> > offset = get_first_obj_offset(page);
> > + pos = offset;
> >
> > spin_lock(&class->lock);
> > - if (!get_zspage_inuse(zspage)) {
> > - ret = -EBUSY;
> > - goto unlock_class;
> > - }
> > -
> > - pos = offset;
> > s_addr = kmap_atomic(page);
> > while (pos < PAGE_SIZE) {
> > head = obj_to_head(page, s_addr + pos);
> >
> >
>
> What about set pos to avoid the loops?
>
> @@ -1997,8 +1997,10 @@ int zs_page_migrate(struct address_space
> *mapping, struct page *newpage,
>
> spin_lock(&class->lock);
> if (!get_zspage_inuse(zspage)) {
> - ret = -EBUSY;
> - goto unlock_class;
> + /* The page is empty.
> + Set "offset" to the end of page.
> + Then the loops of page will be avoided. */
> + offset = PAGE_SIZE;
Good idea. Just a nitpick:
/*
* set "offset" to end of the page so that every loops
* skips unnecessary object scanning.
*/
Thanks!
