| Message ID | 20210323135405.65059-2-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup and fixup for mm/migrate.c | expand |
On 23.03.21 14:54, Miaohe Lin wrote: > The !PageLocked() check is implicitly done in PageMovable(). Remove this > explicit one. > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/migrate.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 47df0df8f21a..facec65c7374 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -145,7 +145,6 @@ void putback_movable_page(struct page *page) > { > struct address_space *mapping; > > - VM_BUG_ON_PAGE(!PageLocked(page), page); > VM_BUG_ON_PAGE(!PageMovable(page), page); > VM_BUG_ON_PAGE(!PageIsolated(page), page); > > Reviewed-by: David Hildenbrand <david@redhat.com>
On Tue, Mar 23, 2021 at 6:54 AM Miaohe Lin <linmiaohe@huawei.com> wrote: > > The !PageLocked() check is implicitly done in PageMovable(). Remove this > explicit one. TBH, I'm a little bit reluctant to have this kind change. If "locked" check is necessary we'd better make it explicit otherwise just remove it. And why not just remove all the 3 VM_BUG_ON_PAGE since putback_movable_page() is just called by putback_movable_pages() and we know the page is locked and both PageMovable and PageIsolated is checked right before calling putback_movable_page(). And you also could make putback_movable_page() static. > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > mm/migrate.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 47df0df8f21a..facec65c7374 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -145,7 +145,6 @@ void putback_movable_page(struct page *page) > { > struct address_space *mapping; > > - VM_BUG_ON_PAGE(!PageLocked(page), page); > VM_BUG_ON_PAGE(!PageMovable(page), page); > VM_BUG_ON_PAGE(!PageIsolated(page), page); > > -- > 2.19.1 >
On 2021/3/24 1:58, Yang Shi wrote: > On Tue, Mar 23, 2021 at 6:54 AM Miaohe Lin <linmiaohe@huawei.com> wrote: >> >> The !PageLocked() check is implicitly done in PageMovable(). Remove this >> explicit one. > > TBH, I'm a little bit reluctant to have this kind change. If "locked" > check is necessary we'd better make it explicit otherwise just remove > it. > > And why not just remove all the 3 VM_BUG_ON_PAGE since > putback_movable_page() is just called by putback_movable_pages() and > we know the page is locked and both PageMovable and PageIsolated is > checked right before calling putback_movable_page(). > > And you also could make putback_movable_page() static. > Sounds good! Many thanks for your advice! >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/migrate.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 47df0df8f21a..facec65c7374 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -145,7 +145,6 @@ void putback_movable_page(struct page *page) >> { >> struct address_space *mapping; >> >> - VM_BUG_ON_PAGE(!PageLocked(page), page); >> VM_BUG_ON_PAGE(!PageMovable(page), page); >> VM_BUG_ON_PAGE(!PageIsolated(page), page); >> >> -- >> 2.19.1 >> > . >
On 2021/3/23 22:27, David Hildenbrand wrote: > On 23.03.21 14:54, Miaohe Lin wrote: >> The !PageLocked() check is implicitly done in PageMovable(). Remove this >> explicit one. >> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> >> --- >> mm/migrate.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 47df0df8f21a..facec65c7374 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -145,7 +145,6 @@ void putback_movable_page(struct page *page) >> { >> struct address_space *mapping; >> - VM_BUG_ON_PAGE(!PageLocked(page), page); >> VM_BUG_ON_PAGE(!PageMovable(page), page); >> VM_BUG_ON_PAGE(!PageIsolated(page), page); >> > > Reviewed-by: David Hildenbrand <david@redhat.com> Many thanks for your review. But I'am going to change this patch, so this Reviewed-by tag may not applies any more. Sorry about it! >
diff --git a/mm/migrate.c b/mm/migrate.c index 47df0df8f21a..facec65c7374 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -145,7 +145,6 @@ void putback_movable_page(struct page *page) { struct address_space *mapping; - VM_BUG_ON_PAGE(!PageLocked(page), page); VM_BUG_ON_PAGE(!PageMovable(page), page); VM_BUG_ON_PAGE(!PageIsolated(page), page);
The !PageLocked() check is implicitly done in PageMovable(). Remove this explicit one. Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> --- mm/migrate.c | 1 - 1 file changed, 1 deletion(-)