From: Oscar Salvador <osalvador@techadventures.net> To: Ross Zwisler <ross.zwisler@linux.intel.com> Cc: Michal Hocko <mhocko@suse.com>, bhe@redhat.com, linux-nvdimm@lists.01.org, Dave Hansen <dave.hansen@linux.intel.com>, LKML <linux-kernel@vger.kernel.org>, pasha.tatashin@oracle.com, Linux MM <linux-mm@kvack.org>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Andrew Morton <akpm@linux-foundation.org>, Vlastimil Babka <vbabka@suse.cz>, osalvador@suse.de Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section Date: Fri, 6 Jul 2018 23:23:27 +0200 [thread overview] Message-ID: <20180706212327.GA10824@techadventures.net> (raw) In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com> On Fri, Jul 06, 2018 at 01:06:58PM -0600, Ross Zwisler wrote: > The following commit in -next: > > commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and > remove check") > > changed how the error handling in sparse_add_one_section() works. > > Previously sparse_index_init() could return -EEXIST, and the function would > continue on happily. 'ret' would get unconditionally overwritten by the > result from sparse_init_one_section() and the error code after the 'out:' > label wouldn't be triggered. My bad, I missed that. > diff --git a/mm/sparse.c b/mm/sparse.c > index 9574113fc745..d254bd2d3289 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, > * plus, it does a kmalloc > */ > ret = sparse_index_init(section_nr, pgdat->node_id); > - if (ret < 0 && ret != -EEXIST) > - return ret; > + if (ret < 0) { > + if (ret == -EEXIST) > + ret = 0; > + else > + return ret; > + } sparse_index_init() can return: -ENOMEM, -EEXIST or 0. So what about this?: diff --git a/mm/sparse.c b/mm/sparse.c index f55e79fda03e..eb188eb6b82d 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, ret = sparse_index_init(section_nr, pgdat->node_id); if (ret < 0 && ret != -EEXIST) return ret; + ret = 0; Does this look more clean? -- Oscar Salvador SUSE L3 _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Oscar Salvador <osalvador@techadventures.net> To: Ross Zwisler <ross.zwisler@linux.intel.com> Cc: pasha.tatashin@oracle.com, linux-nvdimm@lists.01.org, bhe@redhat.com, Dave Hansen <dave.hansen@linux.intel.com>, LKML <linux-kernel@vger.kernel.org>, Linux MM <linux-mm@kvack.org>, Michal Hocko <mhocko@suse.com>, Vlastimil Babka <vbabka@suse.cz>, Andrew Morton <akpm@linux-foundation.org>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, osalvador@suse.de Subject: Re: [PATCH] mm/sparse.c: fix error path in sparse_add_one_section Date: Fri, 6 Jul 2018 23:23:27 +0200 [thread overview] Message-ID: <20180706212327.GA10824@techadventures.net> (raw) In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com> On Fri, Jul 06, 2018 at 01:06:58PM -0600, Ross Zwisler wrote: > The following commit in -next: > > commit 054620849110 ("mm/sparse.c: make sparse_init_one_section void and > remove check") > > changed how the error handling in sparse_add_one_section() works. > > Previously sparse_index_init() could return -EEXIST, and the function would > continue on happily. 'ret' would get unconditionally overwritten by the > result from sparse_init_one_section() and the error code after the 'out:' > label wouldn't be triggered. My bad, I missed that. > diff --git a/mm/sparse.c b/mm/sparse.c > index 9574113fc745..d254bd2d3289 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -753,8 +753,12 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, > * plus, it does a kmalloc > */ > ret = sparse_index_init(section_nr, pgdat->node_id); > - if (ret < 0 && ret != -EEXIST) > - return ret; > + if (ret < 0) { > + if (ret == -EEXIST) > + ret = 0; > + else > + return ret; > + } sparse_index_init() can return: -ENOMEM, -EEXIST or 0. So what about this?: diff --git a/mm/sparse.c b/mm/sparse.c index f55e79fda03e..eb188eb6b82d 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -770,6 +770,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, ret = sparse_index_init(section_nr, pgdat->node_id); if (ret < 0 && ret != -EEXIST) return ret; + ret = 0; Does this look more clean? -- Oscar Salvador SUSE L3
next prev parent reply other threads:[~2018-07-06 21:23 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-02 15:43 [PATCH] mm/sparse: Make sparse_init_one_section void and remove check osalvador 2018-07-02 16:05 ` Michal Hocko 2018-07-02 18:47 ` Pavel Tatashin 2018-07-06 18:23 ` Ross Zwisler 2018-07-06 18:23 ` Ross Zwisler 2018-07-06 19:06 ` [PATCH] mm/sparse.c: fix error path in sparse_add_one_section Ross Zwisler 2018-07-06 19:06 ` Ross Zwisler 2018-07-06 21:23 ` Oscar Salvador [this message] 2018-07-06 21:23 ` Oscar Salvador 2018-07-06 21:54 ` Ross Zwisler 2018-07-06 21:54 ` Ross Zwisler 2018-07-06 21:58 ` Andrew Morton 2018-07-06 21:58 ` Andrew Morton 2018-07-06 21:32 ` Andrew Morton 2018-07-06 21:32 ` Andrew Morton 2018-07-06 22:33 ` [PATCH v2] " Ross Zwisler 2018-07-06 22:33 ` Ross Zwisler 2018-07-07 6:01 ` Oscar Salvador 2018-07-07 6:01 ` Oscar Salvador
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=20180706212327.GA10824@techadventures.net \ --to=osalvador@techadventures.net \ --cc=akpm@linux-foundation.org \ --cc=bhe@redhat.com \ --cc=dave.hansen@linux.intel.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-nvdimm@lists.01.org \ --cc=mhocko@suse.com \ --cc=osalvador@suse.de \ --cc=pasha.tatashin@oracle.com \ --cc=ross.zwisler@linux.intel.com \ --cc=vbabka@suse.cz \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.