linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Baolin Wang <baolin.wang@linux.alibaba.com>
To: Yang Shi <shy828301@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linux MM <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/5] mm: migrate: Remove redundant goto labels
Date: Fri, 6 Aug 2021 11:20:08 +0800	[thread overview]
Message-ID: <98937827-89ec-2a3b-b389-da28f8493cb1@linux.alibaba.com> (raw)
In-Reply-To: <CAHbLzkrqwOp0a_6Vzn29h9qwCiwVSLL6QzsMLfOYTWj7mtkNZg@mail.gmail.com>

Hi Yang,

> On Thu, Aug 5, 2021 at 8:06 AM Baolin Wang
> <baolin.wang@linux.alibaba.com> wrote:
>>
>> Remove redundant goto labels to simplify the code.
> 
> TBH I don't see too much benefit. The "goto" makes the functions have
> a single exit point.

Yes, I agree that the 'goto' statement can make things easier when a 
function exits from multiple locations and some common work such as 
cleanup has to be done, as well as introducing complexity to reading the 
code. So per the coding style documentation, "If there is no cleanup 
needed then just return directly", which can make code more readable I 
think :)

But I have no strong opinion on this, I can drop this patch if you still 
think this is unnecessary. Thanks for your review and comments.

>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>>   mm/migrate.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 0ab364f..ed74fda 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -911,9 +911,8 @@ static int move_to_new_page(struct page *newpage, struct page *page,
>>                   */
>>                  VM_BUG_ON_PAGE(!PageIsolated(page), page);
>>                  if (!PageMovable(page)) {
>> -                       rc = MIGRATEPAGE_SUCCESS;
>>                          __ClearPageIsolated(page);
>> -                       goto out;
>> +                       return MIGRATEPAGE_SUCCESS;
>>                  }
>>
>>                  rc = mapping->a_ops->migratepage(mapping, newpage,
>> @@ -949,7 +948,7 @@ static int move_to_new_page(struct page *newpage, struct page *page,
>>                          flush_dcache_page(newpage);
>>
>>          }
>> -out:
>> +
>>          return rc;
>>   }
>>
>> @@ -2095,11 +2094,10 @@ static struct page *alloc_misplaced_dst_page_thp(struct page *page,
>>          newpage = alloc_pages_node(nid, (GFP_TRANSHUGE_LIGHT | __GFP_THISNODE),
>>                                     HPAGE_PMD_ORDER);
>>          if (!newpage)
>> -               goto out;
>> +               return NULL;
>>
>>          prep_transhuge_page(newpage);
>>
>> -out:
>>          return newpage;
>>   }
>>
>> --
>> 1.8.3.1
>>
>>

  reply	other threads:[~2021-08-06  3:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 15:05 [PATCH 0/5] Some cleanup for page migration Baolin Wang
2021-08-05 15:05 ` [PATCH 1/5] mm: migrate: Move the page count validation to the proper place Baolin Wang
2021-08-05 15:17   ` Matthew Wilcox
2021-08-06  3:07     ` Baolin Wang
2021-08-07  2:02       ` Matthew Wilcox
2021-08-08  2:55         ` Baolin Wang
2021-08-08 10:26           ` Matthew Wilcox
2021-08-08 15:13             ` Baolin Wang
2021-08-08 16:01               ` Matthew Wilcox
2021-08-09  4:19                 ` Baolin Wang
2021-08-05 15:05 ` [PATCH 2/5] mm: migrate: Introduce a local variable to get the number of pages Baolin Wang
2021-08-05 17:42   ` Yang Shi
2021-08-05 15:05 ` [PATCH 3/5] mm: migrate: Fix the incorrect function name in comments Baolin Wang
2021-08-05 17:26   ` Yang Shi
2021-08-09 13:59   ` Alistair Popple
2021-08-05 15:05 ` [PATCH 4/5] mm: migrate: Change to use bool type for 'page_was_mapped' Baolin Wang
2021-08-05 17:34   ` Yang Shi
2021-08-05 15:06 ` [PATCH 5/5] mm: migrate: Remove redundant goto labels Baolin Wang
2021-08-05 19:54   ` Yang Shi
2021-08-06  3:20     ` Baolin Wang [this message]
2021-08-06 17:17       ` Yang Shi
2021-08-08  2:56         ` Baolin Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98937827-89ec-2a3b-b389-da28f8493cb1@linux.alibaba.com \
    --to=baolin.wang@linux.alibaba.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).