On Thu, Apr 30, 2020 at 7:45 PM Daniel Jordan <daniel.m.jordan@xxxxxxxxxx> wrote: > > Hi Alex, > > On Thu, Apr 30, 2020 at 02:43:28PM -0700, Alexander Duyck wrote: > > On 4/30/2020 1:11 PM, Daniel Jordan wrote: > > > padata will soon divide up pfn ranges between threads when parallelizing > > > deferred init, and deferred_init_maxorder() complicates that by using an > > > opaque index in addition to start and end pfns. Move the index outside > > > the function to make splitting the job easier, and simplify the code > > > while at it. > > > > > > deferred_init_maxorder() now always iterates within a single pfn range > > > instead of potentially multiple ranges, and advances start_pfn to the > > > end of that range instead of the max-order block so partial pfn ranges > > > in the block aren't skipped in a later iteration. The section alignment > > > check in deferred_grow_zone() is removed as well since this alignment is > > > no longer guaranteed. It's not clear what value the alignment provided > > > originally. > > > > > > Signed-off-by: Daniel Jordan <daniel.m.jordan@xxxxxxxxxx> > > > > So part of the reason for splitting it up along section aligned boundaries > > was because we already had an existing functionality in deferred_grow_zone > > that was going in and pulling out a section aligned chunk and processing it > > to prepare enough memory for other threads to keep running. I suspect that > > the section alignment was done because normally I believe that is also the > > alignment for memory onlining. > > I think Pavel added that functionality, maybe he could confirm. > > My impression was that the reason deferred_grow_zone aligned the requested > order up to a section was to make enough memory available to avoid being called > on every allocation. > > > With this already breaking things up over multiple threads how does this > > work with deferred_grow_zone? Which thread is it trying to allocate from if > > it needs to allocate some memory for itself? > > I may not be following your question, but deferred_grow_zone doesn't allocate > memory during the multithreading in deferred_init_memmap because the latter > sets first_deferred_pfn so that deferred_grow_zone bails early. It has been a while since I looked at this code so I forgot that deferred_grow_zone is essentially blocked out once we start the per-node init. > > Also what is to prevent a worker from stop deferred_grow_zone from bailing > > out in the middle of a max order page block if there is a hole in the middle > > of the block? > > deferred_grow_zone remains singlethreaded. It could stop in the middle of a > max order block, but it can't run concurrently with deferred_init_memmap, as > per above, so if deferred_init_memmap were to init 'n free the remaining part > of the block, the previous portion would have already been initialized. So we cannot stop in the middle of a max order block. That shouldn't be possible as part of the issue is that the buddy allocator will attempt to access the buddy for the page which could cause issues if it tries to merge the page with one that is not initialized. So if your code supports that then it is definitely broken. That was one of the reasons for all of the variable weirdness in deferred_init_maxorder. I was going through and making certain that while we were initializing the range we were freeing the pages in MAX_ORDER aligned blocks and skipping over whatever reserved blocks were there. Basically it was handling the case where a single MAX_ORDER block could span multiple ranges. On x86 this was all pretty straightforward and I don't believe we needed the code, but I seem to recall there were some other architectures that had more complex memory layouts at the time and that was one of the reasons why I had to be careful to wait until I had processed the full MAX_ORDER block before I could start freeing the pages, otherwise it would start triggering memory corruptions.
