Messages in this thread | | | Date | Wed, 15 Apr 2015 14:24:40 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH 3/4] mm: Gather more PFNs before sending a TLB to flush unmapped pages |
| |
On Wed, Apr 15, 2015 at 01:15:53PM +0100, Mel Gorman wrote: > On Wed, Apr 15, 2015 at 01:42:20PM +0200, Peter Zijlstra wrote: > > On Wed, Apr 15, 2015 at 11:42:55AM +0100, Mel Gorman wrote: > > > +/* > > > + * Use a page to store as many PFNs as possible for batch unmapping. Adjusting > > > + * this trades memory usage for number of IPIs sent > > > + */ > > > +#define BATCH_TLBFLUSH_SIZE \ > > > + ((PAGE_SIZE - sizeof(struct cpumask) - sizeof(unsigned long)) / sizeof(unsigned long)) > > > > > > /* Track pages that require TLB flushes */ > > > struct unmap_batch { > > > + /* Update BATCH_TLBFLUSH_SIZE when adjusting this structure */ > > > struct cpumask cpumask; > > > unsigned long nr_pages; > > > unsigned long pfns[BATCH_TLBFLUSH_SIZE]; > > > > The alternative is something like: > > > > struct unmap_batch { > > struct cpumask cpumask; > > unsigned long nr_pages; > > unsigned long pfnsp[0]; > > }; > > > > #define BATCH_TLBFLUSH_SIZE ((PAGE_SIZE - sizeof(struct unmap_batch)) / sizeof(unsigned long)) > > > > and unconditionally allocate 1 page. This saves you from having to worry > > about the layout of struct unmap_batch. > > True but then I need to calculate the size of the real array so it's > similar in terms of readability. The plus would be that if the structure > changes then the size calculation is not changed but then the allocation > site and the size calculation must be kept in sync. I did not see a clear > win of one approach over the other so flipped a coin.
I'm not seeing your argument, in both your an mine variant the allocation is hard assumed to be 1 page, right? But even then, what's more likely to change, extra members in our struct or growing the allocation to two (or more) pages?
| |