Messages in this thread | | | Subject | Re: [PATCH v4 2nd 1/4] ramoops: Add __ramoops_init_prz() as generic function | From | Mark Salyzyn <> | Date | Tue, 7 Feb 2017 08:05:19 -0800 |
| |
On 02/07/2017 12:51 AM, 岩松信洋 / IWAMATSU,NOBUHIRO wrote: > Hi, > > Thanks for your review. > >> -----Original Message----- >> From: Mark Salyzyn [mailto:salyzyn@android.com] >> Sent: Friday, February 03, 2017 7:13 AM >> To: 岩松信洋 / IWAMATSU,NOBUHIRO; Anton Vorontsov; Colin Cross; Kees Cook; >> Tony Luck >> Cc: linux-kernel@vger.kernel.org; Hiraku Toyooka; 阿口誠司 / AGUCHI, >> SEIJI; Shuah Khan >> Subject: Re: [PATCH v4 2nd 1/4] ramoops: Add __ramoops_init_prz() as generic >> function >> >> On 01/30/2017 05:58 PM, Nobuhiro Iwamatsu wrote: >>> + >>> + if (zap && *paddr + sz - cxt->phys_addr > cxt->size) { >>> + dev_err(dev, "no room for %s mem region (0x%zx@0x%llx) in >> (0x%lx@0x%llx)\n", >>> + name, sz, (unsigned long long)*paddr, >>> + cxt->size, (unsigned long long)cxt->phys_addr); >>> + return -ENOMEM; >>> + } >>> + >> Why not allow this limit check for ramoops_init_prsz call? >> > This code is controlled by flag of zap. > If zap is false, __ramoops_init_prz() is worked same as original ramoops_init_przs(). > > Best regards, > Nobuhiro
I agree, preserve functionality as is. Please consider removing the need for zap && in a future patch, as I _believe_ (untested, and no deep dive) that the limit check is relevant for the zap is false path as well and may have been an oversight.
-- Mark
| |