On Mon 29-10-18 10:34:22, Dan Williams wrote: > On Mon, Oct 29, 2018 at 10:24 AM Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > > > On Mon 29-10-18 10:01:28, Alexander Duyck wrote: > > > On Mon, 2018-10-29 at 17:35 +0100, Michal Hocko wrote: > [..] > > > > You are already doing per-page initialization so I fail to see a larger > > > > unit to operate on. > > > > > > I have a patch that makes it so that we can work at a pageblock level > > > since all of the variables with the exception of only the LRU and page > > > address fields can be precomputed. Doing that is one of the ways I was > > > able to reduce page init to 1/3 to 1/4 of the time it was taking > > > otherwise in the case of deferred page init. > > > > You still have to call set_page_links for each page. But let's assume we > > can do initialization per larger units. Nothing really prevent to hide > > that into constructor as well. > > A constructor / indirect function call makes sense when there are > multiple sub-classes of object initialization, on the table I only see > 3 cases: typical hotplug, base ZONE_DEVICE, ZONE_DEVICE + HMM. I think > we can look to move the HMM special casing out of line, then we're > down to 2. Even at 3 cases we're better off open-coding than a > constructor for such a low number of sub-cases to handle. I do not > foresee more cases arriving, so I struggle to see what the constructor > buys us in terms of code readability / maintainability? I haven't dreamed of ZONE_DEVICE and HMM few years back. But anyway, let me note that I am not in love with callbacks. I find them to be a useful abstraction. I can be convinced (by numbers) that special casing inside the core hotplug code is really beneficial. But let's do that at a single place. All I am arguing against throughout this thread is the memmap_init_zone_device and the whole code duplication just because zone device need something special. -- Michal Hocko SUSE Labs
