All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	hpa@linux.intel.com, linux-mm <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Early boot panic on machine with lots of memory
Date: Wed, 27 Jun 2012 11:13:30 -0700	[thread overview]
Message-ID: <20120627181330.GN15811@google.com> (raw)
In-Reply-To: <CAE9FiQVeJYwpgHjAFp5Q7PazOjeDvN_etrnej987Rc94TjXfAg@mail.gmail.com>

Hello, Yinghai.

Sorry about the delay.  I'm in bug storm somehow. :(

On Fri, Jun 22, 2012 at 07:14:43PM -0700, Yinghai Lu wrote:
> On Fri, Jun 22, 2012 at 12:29 PM, Tejun Heo <tj@kernel.org> wrote:
> > I wish we had a single call - say, memblock_die(), or whatever - so
> > that there's a clear indication that memblock usage is done, but yeah
> > maybe another day.  Will review the patch itself.  BTW, can't you post
> > patches inline anymore?  Attaching is better than corrupt but is still
> > a bit annoying for review.
> 
> please check the three patches:

Heh, reviewing is cumbersome this way but here are my comments.

* "[PATCH] memblock: free allocated memblock_reserved_regions later"
  looks okay to me.

* "[PATCH] memblock: Free allocated memblock.memory.regions" makes me
  wonder whether it would be better to have something like the
  following instead.

  typedef void memblock_free_region_fn_t(unsigned long start, unsigned size);

  void memblock_free_regions(memblock_free_region_fn_t free_fn)
  {
	/* call free_fn() on reserved and memory regions arrays */
	/* clear both structures so that any further usage triggers warning */
  }

* "memblock: Add checking about illegal using memblock".
  Hmm... wouldn't it be better to be less explicit?  I think it's
  adding too much opencoded identical checks.  Maybe implement a
  common check & warning function?

Thanks.

-- 
tejun

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	hpa@linux.intel.com, linux-mm <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: Early boot panic on machine with lots of memory
Date: Wed, 27 Jun 2012 11:13:30 -0700	[thread overview]
Message-ID: <20120627181330.GN15811@google.com> (raw)
In-Reply-To: <CAE9FiQVeJYwpgHjAFp5Q7PazOjeDvN_etrnej987Rc94TjXfAg@mail.gmail.com>

Hello, Yinghai.

Sorry about the delay.  I'm in bug storm somehow. :(

On Fri, Jun 22, 2012 at 07:14:43PM -0700, Yinghai Lu wrote:
> On Fri, Jun 22, 2012 at 12:29 PM, Tejun Heo <tj@kernel.org> wrote:
> > I wish we had a single call - say, memblock_die(), or whatever - so
> > that there's a clear indication that memblock usage is done, but yeah
> > maybe another day.  Will review the patch itself.  BTW, can't you post
> > patches inline anymore?  Attaching is better than corrupt but is still
> > a bit annoying for review.
> 
> please check the three patches:

Heh, reviewing is cumbersome this way but here are my comments.

* "[PATCH] memblock: free allocated memblock_reserved_regions later"
  looks okay to me.

* "[PATCH] memblock: Free allocated memblock.memory.regions" makes me
  wonder whether it would be better to have something like the
  following instead.

  typedef void memblock_free_region_fn_t(unsigned long start, unsigned size);

  void memblock_free_regions(memblock_free_region_fn_t free_fn)
  {
	/* call free_fn() on reserved and memory regions arrays */
	/* clear both structures so that any further usage triggers warning */
  }

* "memblock: Add checking about illegal using memblock".
  Hmm... wouldn't it be better to be less explicit?  I think it's
  adding too much opencoded identical checks.  Maybe implement a
  common check & warning function?

Thanks.

-- 
tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-06-27 18:13 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-13 21:38 Early boot panic on machine with lots of memory Sasha Levin
2012-06-14  3:20 ` Tejun Heo
2012-06-14  3:20   ` Tejun Heo
2012-06-14  9:50   ` Sasha Levin
2012-06-14  9:50     ` Sasha Levin
2012-06-14 20:56     ` Yinghai Lu
2012-06-14 20:56       ` Yinghai Lu
2012-06-14 21:34       ` Sasha Levin
2012-06-14 21:34         ` Sasha Levin
2012-06-14 23:57         ` Yinghai Lu
2012-06-14 23:57           ` Yinghai Lu
2012-06-15  0:59           ` Sasha Levin
2012-06-15  0:59             ` Sasha Levin
2012-06-15  0:59             ` Sasha Levin
2012-06-15  2:21             ` Yinghai Lu
2012-06-15  2:21               ` Yinghai Lu
2012-06-15  7:41               ` Sasha Levin
2012-06-15  7:41                 ` Sasha Levin
2012-06-18 22:32     ` Tejun Heo
2012-06-18 22:32       ` Tejun Heo
2012-06-18 22:50       ` Sasha Levin
2012-06-18 22:50         ` Sasha Levin
2012-06-19  4:11         ` Gavin Shan
2012-06-19  4:11           ` Gavin Shan
2012-06-19  5:43           ` Yinghai Lu
2012-06-19  5:43             ` Yinghai Lu
2012-06-19  6:09             ` Gavin Shan
2012-06-19  6:09               ` Gavin Shan
2012-06-19 18:12               ` Yinghai Lu
2012-06-19 18:12                 ` Yinghai Lu
2012-06-19 21:20           ` Tejun Heo
2012-06-19 21:20             ` Tejun Heo
2012-06-19 21:26             ` Tejun Heo
2012-06-19 21:26               ` Tejun Heo
2012-06-20  2:57               ` Yinghai Lu
2012-06-21 20:17                 ` Tejun Heo
2012-06-21 20:17                   ` Tejun Heo
2012-06-22  1:47                   ` Yinghai Lu
2012-06-22  1:58                     ` Yinghai Lu
2012-06-22 18:51                     ` Tejun Heo
2012-06-22 18:51                       ` Tejun Heo
2012-06-22 19:23                       ` Yinghai Lu
2012-06-22 19:23                         ` Yinghai Lu
2012-06-22 19:29                         ` Tejun Heo
2012-06-22 19:29                           ` Tejun Heo
2012-06-22 20:01                           ` Yinghai Lu
2012-06-22 20:01                             ` Yinghai Lu
2012-06-22 20:14                             ` Tejun Heo
2012-06-22 20:14                               ` Tejun Heo
2012-06-22 20:23                               ` Yinghai Lu
2012-06-22 20:23                                 ` Yinghai Lu
2012-06-23  2:14                           ` Yinghai Lu
2012-06-27 18:13                             ` Tejun Heo [this message]
2012-06-27 18:13                               ` Tejun Heo
2012-06-27 19:22                               ` Yinghai Lu
2012-06-27 19:22                                 ` Yinghai Lu
2012-06-27 19:26                                 ` Tejun Heo
2012-06-27 19:26                                   ` Tejun Heo
2012-06-27 21:15                                   ` Yinghai Lu
2012-06-29 18:27                                     ` [PATCH for -3.5] memblock: free allocated memblock_reserved_regions later Yinghai Lu
2012-06-29 18:27                                       ` Yinghai Lu
2012-06-29 18:32                                       ` Tejun Heo
2012-06-29 18:32                                         ` Tejun Heo
2012-06-29 18:38                                         ` Yinghai Lu
2012-06-29 18:38                                           ` Yinghai Lu
2012-06-21 20:19             ` Early boot panic on machine with lots of memory Tejun Heo
2012-06-21 20:19               ` Tejun Heo
2012-06-22 10:29               ` Sasha Levin
2012-06-22 10:29                 ` Sasha Levin
2012-06-22 18:15                 ` Yinghai Lu
2012-06-22 18:15                   ` Yinghai Lu

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=20120627181330.GN15811@google.com \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=hpa@linux.intel.com \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shangw@linux.vnet.ibm.com \
    --cc=yinghai@kernel.org \
    /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.