Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <28718037-ccf1-c36b-4eb6-716c518ca98f@igalia.com>
Date:   Wed, 28 Jun 2023 15:12:10 -0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] pstore: ramoops: support pmsg size larger than kmalloc
 limitation
Content-Language: en-US
To:     Yuxiao Zhang <yuxiaozhang@google.com>
Cc:     Kees Cook <keescook@chromium.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Tony Luck <tony.luck@intel.com>,
        linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
        wak@google.com
References: <2023062832-barrier-strenuous-9499@gregkh>
 <20230628171055.1312737-1-yuxiaozhang@google.com>
From:   "Guilherme G. Piccoli" <gpiccoli@igalia.com>
In-Reply-To: <20230628171055.1312737-1-yuxiaozhang@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 28/06/2023 19:10, Yuxiao Zhang wrote:
> Thanks for reviewing the patch.
> 
> On 28 Jun 2023 07:30:16 +0200, Greg KH wrote:
>> What is that max size?
> 
> The max size is arch dependent, it should be 2^(PAGE_SIZE+MAX_ORDER). In our environment it is 4M.
> 
>> what in-kernel user is hitting this in the pstore implementation?
> 
> We are trying to use pmsg to hold a core dump file, so we have pmsg-size=32M and thus hit this issue.
> 
> Other than us, here is one I found that trying to save dmesg beyond kmalloc limitaton:
> https://lore.kernel.org/lkml/b2d66d9f-15a6-415c-2485-44649027a1d5@igalia.com/T/

Hi Yuxiao Zhang, thanks for the improvement! And also, thanks for
mentioning the thread above - I tested your patch today and was soon to
write you this message heh

So, first of all, the patch works for the Steam Deck case - kernel 6.4
with or without your patch behaved the same, i.e., pstore/ramoops worked
and it was possible to collect the dmesg.

But when I tried to increase the record size in ramoops, I got this
error: https://termbin.com/b12e

This is the same error as mentioned in the thread above. And it happens
when I try to bump the record size to 4MB, the biggest working value is
still 2MB. Maybe a missing spot, which remained using kmalloc() instead
of the virtual variant?

Also - Kees certainly knows that way better, but - are we 100% sure that
the region for pstore can be non-contiguous? For some reason, I always
thought this was a requirement - grepping the code, I found this
(wrong?) comment:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/pstore/zone.c#n3

Cheers,


Guilherme