From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Date: Tue, 19 Jul 2016 14:27:54 +0100 From: Mark Rutland Subject: Re: [PATCH v22 1/8] arm64: kdump: reserve memory for crash dump kernel Message-ID: <20160719132736.GB21007@leverpostej> References: <20160712050514.22307-1-takahiro.akashi@linaro.org> <20160712050514.22307-2-takahiro.akashi@linaro.org> <20160719093906.GA20732@arm.com> <20160719102815.GE20774@linaro.org> <20160719104103.GB20990@arm.com> <1468932537.27473.6.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1468932537.27473.6.camel@redhat.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Mark Salter Cc: Pratyush Anand , geoff@infradead.org, catalin.marinas@arm.com, will.deacon@arm.com, AKASHI Takahiro , robh+dt@kernel.org, james.morse@arm.com, bauerman@linux.vnet.ibm.com, Dennis Chen , nd@arm.com, dyoung@redhat.com, kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org T24gVHVlLCBKdWwgMTksIDIwMTYgYXQgMDg6NDg6NTdBTSAtMDQwMCwgTWFyayBTYWx0ZXIgd3Jv dGU6Cj4gT24gVHVlLCAyMDE2LTA3LTE5IGF0IDE4OjQxICswODAwLCBEZW5uaXMgQ2hlbiB3cm90 ZToKPiA+IE9uIFR1ZSwgSnVsIDE5LCAyMDE2IGF0IDA3OjI4OjE2UE0gKzA5MDAsIEFLQVNISSBU YWthaGlybyB3cm90ZToKPiA+ID4gT24gVHVlLCBKdWwgMTksIDIwMTYgYXQgMDU6Mzk6MDdQTSAr MDgwMCwgRGVubmlzIENoZW4gd3JvdGU6Cj4gPiA+ID4gT24gVHVlLCBKdWwgMTIsIDIwMTYgYXQg MDI6MDU6MDdQTSArMDkwMCwgQUtBU0hJIFRha2FoaXJvIHdyb3RlOgo+ID4gPiA+ID4gKy8qCj4g PiA+ID4gPiArICogcmVzZXJ2ZV9jcmFzaGtlcm5lbCgpIC0gcmVzZXJ2ZXMgbWVtb3J5IGZvciBj cmFzaCBrZXJuZWwKPiA+ID4gPiA+ICsgKgo+ID4gPiA+ID4gKyAqIFRoaXMgZnVuY3Rpb24gcmVz ZXJ2ZXMgbWVtb3J5IGFyZWEgZ2l2ZW4gaW4gImNyYXNoa2VybmVsPSIga2VybmVsIGNvbW1hbmQK PiA+ID4gPiA+ICsgKiBsaW5lIHBhcmFtZXRlci4gVGhlIG1lbW9yeSByZXNlcnZlZCBpcyB1c2Vk IGJ5IGR1bXAgY2FwdHVyZSBrZXJuZWwgd2hlbgo+ID4gPiA+ID4gKyAqIHByaW1hcnkga2VybmVs IGlzIGNyYXNoaW5nLgo+ID4gPiA+ID4gKyAqLwo+ID4gPiA+ID4gK3N0YXRpYyB2b2lkIF9faW5p dCByZXNlcnZlX2NyYXNoa2VybmVsKHZvaWQpCj4gPiA+ID4gPiArewo+ID4gPiA+ID4gK8KgwqDC oMKgwqBpbnQgcmV0Owo+ID4gPiA+ID4gKwo+ID4gPiA+ID4gK8KgwqDCoMKgwqByZXQgPSBwYXJz ZV9jcmFzaGtlcm5lbChib290X2NvbW1hbmRfbGluZSwgbWVtYmxvY2tfcGh5c19tZW1fc2l6ZSgp LAo+ID4gPiA+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqAmY3Jhc2hfc2l6ZSwgJmNyYXNoX2Jhc2UpOwo+ID4gPiA+ID4gK8KgwqDC oMKgwqAvKiBubyBjcmFzaGtlcm5lbD0gb3IgaW52YWxpZCB2YWx1ZSBzcGVjaWZpZWQgKi8KPiA+ ID4gPiA+ICvCoMKgwqDCoMKgaWYgKHJldCB8fCAhY3Jhc2hfc2l6ZSkKPiA+ID4gPiA+ICvCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJldHVybjsKPiA+ID4gPiA+ICsKPiA+ID4gPiA+ICvCoMKg wqDCoMKgaWYgKGNyYXNoX2Jhc2UgPT0gMCkgewo+ID4gPiA+ID4gK8KgwqDCoMKgwqDCoMKgwqDC oMKgwqDCoMKgLyogQ3VycmVudCBhcm02NCBib290IHByb3RvY29sIHJlcXVpcmVzIDJNQiBhbGln bm1lbnQgKi8KPiA+ID4gPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGNyYXNoX2Jhc2Ug PSBtZW1ibG9ja19maW5kX2luX3JhbmdlKDAsCj4gPiA+ID4gPiArwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoE1FTUJMT0NLX0FMTE9DX0FD Q0VTU0lCTEUsIGNyYXNoX3NpemUsIFNaXzJNKTsKPiA+ID4gPiA+ICvCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoGlmIChjcmFzaF9iYXNlID09IDApIHsKPiA+ID4gPiA+ICvCoMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBwcl93YXJuKCJVbmFibGUgdG8gYWxsb2NhdGUg Y3Jhc2hrZXJuZWwgKHNpemU6JWxseClcbiIsCj4gPiA+ID4gPiArwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGNyYXNoX3NpemUpOwo+ID4g PiA+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJldHVybjsK PiA+ID4gPiA+ICvCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoH0KPiA+ID4gPiA+ICvCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoG1lbWJsb2NrX3Jlc2VydmUoY3Jhc2hfYmFzZSwgY3Jhc2hfc2l6 ZSk7Cj4gPiA+ID4gPiAKPiA+ID4gPiBJIGFtIG5vdCBwcmV0dHkgc3VyZSB0aGUgY29udGV4dCBo ZXJlLCBidXQKPiA+ID4gPiBjYW4gd2UgdXNlIGJlbG93IGNvZGUgcGllY2UgaW5zdGVhZCBvZiB0 aGUgYWJvdmUgbGluZXM/Cj4gPiA+ID4gwqDCoMKgwqDCoMKgwqDCoGlmIChjcmFzaF9iYXNlID09 IDApCj4gPiA+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBtZW1ibG9ja19hbGxv YyhjcmFzaF9zaXplLCBTWl8yTSk7Cj4gPiA+IEVpdGhlciB3b3VsZCBiZSBmaW5lIGhlcmUuCj4g PiA+IAo+ID4gSGVsbG8gQUtBU0hJLCBtYXliZSB5b3UgY2FuIHN1Y2NlZWQgdG8gZmluZCB0aGUg YmFzZSB3aXRoIG1lbWJsb2NrX2ZpbmRfaW5fcmFuZ2UoKSzCoAo+ID4gYnV0IHRoYXQgZG9lc24n dCBtZWFuIHlvdSB3aWxsIGFsc28gc3VjY2VlZCB0byByZXNlcnZlIHRoZW0gd2l0aCBtZW1ibG9j a19yZXNlcnZlIGZvbGxvd2VkLgo+IAo+IFdlIGF2b2lkIG1lbWJsb2NrX2FsbG9jKCkgaGVyZSBi ZWNhdXNlIGl0IHBhbmljcyBvbiBmYWlsdXJlLiBUaGlzIGNvdWxkIGhhcHBlbgo+IGlmIHVzZXIg YXNrcyBmb3IgYW4gdW51c3VhbGx5IGxhcmdlIGNyYXNoa2VybmVsIHNpemUuIEJldHRlciB0byBw cmludCBhIG1lc3NhZ2UKPiBhbmQga2VlcCBib290aW5nLiBDaGVja2luZyB0aGUgcmV0dXJuIHZh bHVlIG9mIG1lbWJsb2NrX3Jlc2VydmUoKSBzZWVtcyBsaWtlIGEKPiBnb29kIHRoaW5nIHRvIGRv IHRob3VnaC4KCkFub3RoZXIgb3B0aW9uIHdvdWxkIGJlIHRvIGFkZCBhIG1lbWJsb2NrX3RyeV9h bGxvYygpIGZ1bmN0aW9uIHRvIHRoZQptZW1ibG9jayBBUEksIHdoaWNoIGluIGNhc2Ugb2YgZmFp bHVyZSByZXR1cm5zIDAgcmF0aGVyIHRoYW4gdHJpZ2dlcmluZwphIHBhbmljKCkuIFdlJ2Qgc3Rp bGwgaGF2ZSB0byBjaGVjayB0aGUgcmV0dXJuIHZhbHVlLCBidXQgYWxsIHRoZQptZW1ibG9jayBt YW5pcHVsYXRpb24gd291bGQgYmUgaW4gb25lIHBsYWNlLgoKSXQgbG9va3MgbGlrZSBhZGRpbmcg dGhhdCBpcyBmYWlybHkgc2ltcGxlOgoKcGh5c19hZGRyX3QgX19pbml0IG1lbWJsb2NrX3RyeV9h bGxvYyhwaHlzX2FkZHJfdCBzaXplLCBwaHlzX2FkZHJfdCBhbGlnbikKewoJcmV0dXJuIF9fbWVt YmxvY2tfYWxsb2NfYmFzZShzaXplLCBhbGlnbiwgTUVNQkxPQ0tfQUxMT0NfQUNDRVNTSUJMRSk7 Cn0KClRoYW5rcywKTWFyay4KCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCmtleGVjIG1haWxpbmcgbGlzdAprZXhlY0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0 dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8va2V4ZWMK From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.rutland@arm.com (Mark Rutland) Date: Tue, 19 Jul 2016 14:27:54 +0100 Subject: [PATCH v22 1/8] arm64: kdump: reserve memory for crash dump kernel In-Reply-To: <1468932537.27473.6.camel@redhat.com> References: <20160712050514.22307-1-takahiro.akashi@linaro.org> <20160712050514.22307-2-takahiro.akashi@linaro.org> <20160719093906.GA20732@arm.com> <20160719102815.GE20774@linaro.org> <20160719104103.GB20990@arm.com> <1468932537.27473.6.camel@redhat.com> Message-ID: <20160719132736.GB21007@leverpostej> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, Jul 19, 2016 at 08:48:57AM -0400, Mark Salter wrote: > On Tue, 2016-07-19 at 18:41 +0800, Dennis Chen wrote: > > On Tue, Jul 19, 2016 at 07:28:16PM +0900, AKASHI Takahiro wrote: > > > On Tue, Jul 19, 2016 at 05:39:07PM +0800, Dennis Chen wrote: > > > > On Tue, Jul 12, 2016 at 02:05:07PM +0900, AKASHI Takahiro wrote: > > > > > +/* > > > > > + * reserve_crashkernel() - reserves memory for crash kernel > > > > > + * > > > > > + * This function reserves memory area given in "crashkernel=" kernel command > > > > > + * line parameter. The memory reserved is used by dump capture kernel when > > > > > + * primary kernel is crashing. > > > > > + */ > > > > > +static void __init reserve_crashkernel(void) > > > > > +{ > > > > > +?????int ret; > > > > > + > > > > > +?????ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(), > > > > > +?????????????????????????????&crash_size, &crash_base); > > > > > +?????/* no crashkernel= or invalid value specified */ > > > > > +?????if (ret || !crash_size) > > > > > +?????????????return; > > > > > + > > > > > +?????if (crash_base == 0) { > > > > > +?????????????/* Current arm64 boot protocol requires 2MB alignment */ > > > > > +?????????????crash_base = memblock_find_in_range(0, > > > > > +?????????????????????????????MEMBLOCK_ALLOC_ACCESSIBLE, crash_size, SZ_2M); > > > > > +?????????????if (crash_base == 0) { > > > > > +?????????????????????pr_warn("Unable to allocate crashkernel (size:%llx)\n", > > > > > +?????????????????????????????crash_size); > > > > > +?????????????????????return; > > > > > +?????????????} > > > > > +?????????????memblock_reserve(crash_base, crash_size); > > > > > > > > > I am not pretty sure the context here, but > > > > can we use below code piece instead of the above lines? > > > > ????????if (crash_base == 0) > > > > ????????????????memblock_alloc(crash_size, SZ_2M); > > > Either would be fine here. > > > > > Hello AKASHI, maybe you can succeed to find the base with memblock_find_in_range(),? > > but that doesn't mean you will also succeed to reserve them with memblock_reserve followed. > > We avoid memblock_alloc() here because it panics on failure. This could happen > if user asks for an unusually large crashkernel size. Better to print a message > and keep booting. Checking the return value of memblock_reserve() seems like a > good thing to do though. Another option would be to add a memblock_try_alloc() function to the memblock API, which in case of failure returns 0 rather than triggering a panic(). We'd still have to check the return value, but all the memblock manipulation would be in one place. It looks like adding that is fairly simple: phys_addr_t __init memblock_try_alloc(phys_addr_t size, phys_addr_t align) { return __memblock_alloc_base(size, align, MEMBLOCK_ALLOC_ACCESSIBLE); } Thanks, Mark.