| Message ID | 20220304093409.25829-2-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | A few cleanup and fixup patches for migration | expand |
On Fri, Mar 4, 2022 at 5:35 PM Miaohe Lin <linmiaohe@huawei.com> wrote: > > When mapping_locked is true, TTU_RMAP_LOCKED is always set to ttu. We can > check ttu instead so mapping_locked can be removed. And ttu is either 0 > or TTU_RMAP_LOCKED now. Change '|=' to '=' to reflect this. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> Reviewed-by: Muchun Song <songmuchun@bytedance.com> Thanks.
Miaohe Lin <linmiaohe@huawei.com> writes: > When mapping_locked is true, TTU_RMAP_LOCKED is always set to ttu. We can > check ttu instead so mapping_locked can be removed. And ttu is either 0 > or TTU_RMAP_LOCKED now. Change '|=' to '=' to reflect this. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/migrate.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 42e3a944f35c..50bc62d85eaf 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1230,7 +1230,6 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, > goto put_anon; > > if (page_mapped(hpage)) { > - bool mapping_locked = false; > enum ttu_flags ttu = 0; > > if (!PageAnon(hpage)) { > @@ -1244,14 +1243,13 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, > if (unlikely(!mapping)) > goto unlock_put_anon; > > - mapping_locked = true; > - ttu |= TTU_RMAP_LOCKED; > + ttu = TTU_RMAP_LOCKED; > } > > try_to_migrate(src, ttu); > page_was_mapped = 1; > > - if (mapping_locked) > + if (ttu & TTU_RMAP_LOCKED) > i_mmap_unlock_write(mapping); > } I think this is a code style issue. The original code looks more readable for me. Best Regards, Huang, Ying
On 2022/3/7 10:00, Huang, Ying wrote: > Miaohe Lin <linmiaohe@huawei.com> writes: > >> When mapping_locked is true, TTU_RMAP_LOCKED is always set to ttu. We can >> check ttu instead so mapping_locked can be removed. And ttu is either 0 >> or TTU_RMAP_LOCKED now. Change '|=' to '=' to reflect this. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/migrate.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 42e3a944f35c..50bc62d85eaf 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1230,7 +1230,6 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, >> goto put_anon; >> >> if (page_mapped(hpage)) { >> - bool mapping_locked = false; >> enum ttu_flags ttu = 0; >> >> if (!PageAnon(hpage)) { >> @@ -1244,14 +1243,13 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, >> if (unlikely(!mapping)) >> goto unlock_put_anon; >> >> - mapping_locked = true; >> - ttu |= TTU_RMAP_LOCKED; >> + ttu = TTU_RMAP_LOCKED; >> } >> >> try_to_migrate(src, ttu); >> page_was_mapped = 1; >> >> - if (mapping_locked) >> + if (ttu & TTU_RMAP_LOCKED) >> i_mmap_unlock_write(mapping); >> } > > I think this is a code style issue. The original code looks more > readable for me. > I think "ttu |= TTU_RMAP_LOCKED;" is the product of the code changing. And (ttu & TTU_RMAP_LOCKED) can do the right thing as mapping_locked while removing the page_was_mapped. But if you insist on this, I will drop this patch. Thanks. > Best Regards, > Huang, Ying > > . >
diff --git a/mm/migrate.c b/mm/migrate.c index 42e3a944f35c..50bc62d85eaf 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1230,7 +1230,6 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, goto put_anon; if (page_mapped(hpage)) { - bool mapping_locked = false; enum ttu_flags ttu = 0; if (!PageAnon(hpage)) { @@ -1244,14 +1243,13 @@ static int unmap_and_move_huge_page(new_page_t get_new_page, if (unlikely(!mapping)) goto unlock_put_anon; - mapping_locked = true; - ttu |= TTU_RMAP_LOCKED; + ttu = TTU_RMAP_LOCKED; } try_to_migrate(src, ttu); page_was_mapped = 1; - if (mapping_locked) + if (ttu & TTU_RMAP_LOCKED) i_mmap_unlock_write(mapping); }
When mapping_locked is true, TTU_RMAP_LOCKED is always set to ttu. We can check ttu instead so mapping_locked can be removed. And ttu is either 0 or TTU_RMAP_LOCKED now. Change '|=' to '=' to reflect this. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/migrate.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)