Hi Jan
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Monday, July 5, 2021 3:51 PM
> To: Penny Zheng <penny.zh...@arm.com>
> Cc: Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen
> <wei.c...@arm.com>; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; Julien Grall <jul...@xen.org>
> Subject: Re: [PATCH 4/9] xen/arm: static memory initialization
>
> On 05.07.2021 09:14, Penny Zheng wrote:
> >> From: Penny Zheng
> >> Sent: Monday, July 5, 2021 1:22 PM
> >>
> >>> From: Julien Grall <jul...@xen.org>
> >>> Sent: Thursday, July 1, 2021 1:46 AM
> >>>
> >>> On 10/06/2021 10:35, Jan Beulich wrote:
> >>>> On 07.06.2021 04:43, Penny Zheng wrote:
> >>>>> @@ -1512,6 +1530,38 @@ static void free_heap_pages(
> >>>>> spin_unlock(&heap_lock);
> >>>>> }
> >>>>>
> >>>>> +#ifdef CONFIG_STATIC_ALLOCATION
> >>>>> +/* Equivalent of free_heap_pages to free nr_mfns pages of static
> >>>>> +memory. */ void __init free_staticmem_pages(struct page_info *pg,
> >>> unsigned long nr_mfns,
> >>>>> + bool need_scrub) {
> >>>>> + mfn_t mfn = page_to_mfn(pg);
> >>>>> + unsigned long i;
> >>>>> +
> >>>>> + for ( i = 0; i < nr_mfns; i++ )
> >>>>> + {
> >>>>> + switch ( pg[i].count_info & PGC_state )
> >>>>> + {
> >>>>> + case PGC_state_inuse:
> >>>>> + BUG_ON(pg[i].count_info & PGC_broken);
> >>>>> + /* Mark it free and reserved. */
> >>>>> + pg[i].count_info = PGC_state_free | PGC_reserved;
> >>>>> + break;
> >>>>> +
> >>>>> + default:
> >>>>> + printk(XENLOG_ERR
> >>>>> + "Page state shall be only in PGC_state_inuse. "
> >>>>
> >>>> Why? A page (static or not) can become broken while in use. IOW I
> >>>> don't think you can avoid handling PGC_state_offlining here. At
> >>>> which point this code will match free_heap_pages()'es, and hence
> >>>> likely will want folding as well.
> >>>>
> >>
> >> Yeah, I was following the logic in free_heap_pages.
> >> Hmmm, I could not think of any scenario that will lead to
> >> PGC_state_offlining, that's why I was not including it at the first place.
> >> For broken issue, tbh, I just copy the bug_on from free_heap_pages,
> >> after quite a time thinking, I also could not find any scenario when
> >> a page(static or
> >> not) can become broken while in use. ;/
>
> I'm, afraid I don't understand. Using page_to_mfn(), expensive or not, in
> ASSERT() is quite fine. The (expensive) expression won't be evaluated in
> release builds. This is specifically different from BUG_ON().
>
Thanks for the explanation. 😉
I'll use the ASSERT() to do the check.
> Jan
Cheers
Penny Zheng
