All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: osalvador@techadventures.net, 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>,
	Michal Hocko <mhocko@suse.com>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	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 14:32:25 -0700	[thread overview]
Message-ID: <20180706143225.1cf9569f240dccf91bdc3788@linux-foundation.org> (raw)
In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com>

On Fri,  6 Jul 2018 13:06:58 -0600 Ross Zwisler <ross.zwisler@linux.intel.com> wrote:

> 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.
> 
> With the above referenced commit, though, an -EEXIST error return from
> sparse_index_init() now takes us through the function and into the error
> case after 'out:'.  This eventually causes a kernel BUG, probably because
> we've just freed a memory section that we successfully set up and marked as
> present:

Thanks.

And gee it would be nice if some of this code was commented.  I
*assume* what's happening with that -EEXIST is that
sparse_add_one_section() is discovering that the root mem_section was
already initialized so things are OK.  Maybe.  My mind-reading skills
aren't so good on Fridays.

And sparse_index_init() sure looks like it needs locking to avoid races
around mem_section[root].  Or perhaps we're known to be single-threaded
here.
_______________________________________________
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: Andrew Morton <akpm@linux-foundation.org>
To: Ross Zwisler <ross.zwisler@linux.intel.com>
Cc: pasha.tatashin@oracle.com, linux-nvdimm@lists.01.org,
	osalvador@techadventures.net, 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>,
	"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 14:32:25 -0700	[thread overview]
Message-ID: <20180706143225.1cf9569f240dccf91bdc3788@linux-foundation.org> (raw)
In-Reply-To: <20180706190658.6873-1-ross.zwisler@linux.intel.com>

On Fri,  6 Jul 2018 13:06:58 -0600 Ross Zwisler <ross.zwisler@linux.intel.com> wrote:

> 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.
> 
> With the above referenced commit, though, an -EEXIST error return from
> sparse_index_init() now takes us through the function and into the error
> case after 'out:'.  This eventually causes a kernel BUG, probably because
> we've just freed a memory section that we successfully set up and marked as
> present:

Thanks.

And gee it would be nice if some of this code was commented.  I
*assume* what's happening with that -EEXIST is that
sparse_add_one_section() is discovering that the root mem_section was
already initialized so things are OK.  Maybe.  My mind-reading skills
aren't so good on Fridays.

And sparse_index_init() sure looks like it needs locking to avoid races
around mem_section[root].  Or perhaps we're known to be single-threaded
here.

  parent reply	other threads:[~2018-07-06 21:32 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
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 [this message]
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=20180706143225.1cf9569f240dccf91bdc3788@linux-foundation.org \
    --to=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=osalvador@techadventures.net \
    --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: link
Be 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.