Messages in this thread | | | Date | Wed, 11 May 2022 16:11:28 -0700 | From | Minchan Kim <> | Subject | Re: [PATCH] zsmalloc: Fix races between asynchronous zspage free and page migration |
| |
On Wed, May 11, 2022 at 02:45:30PM -0700, Sultan Alsawaf wrote: > On Wed, May 11, 2022 at 02:07:19PM -0700, Minchan Kim wrote: > > Then, how about this? > > Your proposal is completely wrong still. My original patch is fine; we can stick > with that. > > > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > > index 9152fbde33b5..2f205c18aee4 100644 > > --- a/mm/zsmalloc.c > > +++ b/mm/zsmalloc.c > > @@ -1716,12 +1716,31 @@ static enum fullness_group putback_zspage(struct size_class *class, > > * To prevent zspage destroy during migration, zspage freeing should > > * hold locks of all pages in the zspage. > > */ > > -static void lock_zspage(struct zspage *zspage) > > +static void lock_zspage(struct zs_pool *pool, struct zspage *zspage) > > { > > - struct page *page = get_first_page(zspage); > > - > > + struct page *page; > > + int nr_locked; > > + struct page *locked_pages[ZS_MAX_PAGES_PER_ZSPAGE]; > > + struct address_space *mapping; > > +retry: > > + nr_locked = 0; > > + memset(locked_pages, 0, sizeof(struct page) * ARRAY_SIZE(locked_pages)); > > This memset() zeroes out memory past the end of the array because it is an array > of pointers, not an array of page structs; the sizeof() is incorrect. > > > + page = get_first_page(zspage); > > You can't use get_first_page() outside of the migrate lock. > > > do { > > lock_page(page); > > You can't lock a page that you don't own.
That's key point what my idea was wrong.
Thanks for correction!
| |