Hi jan
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Thursday, May 5, 2022 3:47 PM
> To: Penny Zheng <penny.zh...@arm.com>
> Cc: Wei Chen <wei.c...@arm.com>; Henry Wang <henry.w...@arm.com>;
> Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap
> <george.dun...@citrix.com>; Julien Grall <jul...@xen.org>; Stefano
> Stabellini <sstabell...@kernel.org>; Wei Liu <w...@xen.org>; xen-
> de...@lists.xenproject.org
> Subject: Re: [PATCH v3 6/6] xen: retrieve reserved pages on
> populate_physmap
>
> On 05.05.2022 08:24, Penny Zheng wrote:
> >> From: Jan Beulich <jbeul...@suse.com>
> >> Sent: Wednesday, May 4, 2022 9:45 PM
> >>
> >> On 27.04.2022 11:27, Penny Zheng wrote:
> >>> #else
> >>> void free_staticmem_pages(struct page_info *pg, unsigned long
> nr_mfns,
> >>> bool need_scrub) {
> >>> ASSERT_UNREACHABLE();
> >>> }
> >>> +
> >>> +int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
> >>> + unsigned int nr_mfns, unsigned
> >>> +int
> >>> +memflags) {
> >>> + ASSERT_UNREACHABLE();
> >>> +}
> >>
> >> I can't spot a caller of this one outside of suitable #ifdef. Also
> >> the __init here looks wrong and you look to have missed dropping it from
> the real function.
> >>
> >>> +mfn_t acquire_reserved_page(struct domain *d, unsigned int
> >>> +memflags) {
> >>> + ASSERT_UNREACHABLE();
> >>> +}
> >>> #endif
> >>
> >> For this one I'd again expect CSE to leave no callers, just like in
> >> the earlier patch. Or am I overlooking anything?
> >>
> >
> > In acquire_reserved_page, I've use a few CONFIG_STATIC_MEMORY-only
> > variables, like
> > d->resv_page_list, so I'd expect to let acquire_reserved_page guarded
> > d->by CONFIG_STATIC_MEMORY
> > too and provide the stub function here to avoid compilation error
> when !CONFIG_STATIC_MEMORY.
>
> A compilation error should only result if there's no declaration of the
> function. I didn't suggest to remove that. A missing definition would only be
> noticed when linking, but CSE should result in no reference to the function in
> the first place. Just like was suggested for the earlier patch. And as opposed
> to the call site optimization the compiler can do, with -ffunction-sections
> there's no way for the linker to eliminate the dead stub function.
>
Sure, plz correct me if I understand wrongly:
Maybe here I should use #define xxx to do the declaration, and it will also
avoid bringing dead stub function. Something like:
#define free_staticmem_pages(pg, nr_mfns, need_scrub) ((void)(pg),
(void)(nr_mfns), (void)(need_scrub))
And
#define acquire_reserved_page(d, memflags) (INVALID_MFN)
> Jan
