On Tue, Jun 09, 2026 at 11:53:20AM -0400, Michael S. Tsirkin wrote: > --- a/mm/page_reporting.c > +++ b/mm/page_reporting.c > @@ -174,10 +174,10 @@ page_reporting_cycle(struct page_reporting_dev_info > *prdev, struct zone *zone, > * list processed. This should result in us reporting all pages on > * an idle system in about 30 seconds. > * > - * The division here should be cheap since PAGE_REPORTING_CAPACITY > - * should always be a power of 2. > + * The division here uses integer division; capacity need > + * not be a power of 2. > */ > - budget = DIV_ROUND_UP(area->nr_free, PAGE_REPORTING_CAPACITY * 16); > + budget = DIV_ROUND_UP(area->nr_free, prdev->capacity * 16); >
Initial look - is there a div-by-0 here? I noticed the old check prevents this from being (0 * 16), but i don't see (on first pass) the same check anywhere. Unless this line below always forces the above to be a PAGE_REPORTING_CAPCAITY if it's set to 0. > + if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY) > + prdev->capacity = PAGE_REPORTING_CAPACITY; > + It's worth making this corner condition a little more obvious. The code intends for if (capacity == 0) capacity = PAGE_REPORTING_CAPACITY but that's not reflected in the changelog as a default value. When happens if a driver sets (capacity=0) either on purpose (???) or because there's a bug (???) and then page_reporting.c forces it up to 32? There's something to improve here. ~Gregory
