On 1/30/2026 4:12 PM, Gregory Price wrote: > On Fri, Jan 30, 2026 at 03:27:12PM -0600, Cheatham, Benjamin wrote: >> On 1/29/2026 3:04 PM, Gregory Price wrote: >>> In the current kmem driver binding process, the only way for users >>> to define hotplug policy is via a build-time option, or by not >>> onlining memory by default and setting each individual memory block >>> online after hotplug occurs. We can solve this with a configuration >>> step between region-probe and dax-probe. >>> >>> Add the infrastructure for a two-stage driver binding for kmem-mode >>> dax regions. The cxl_dax_kmem_region driver probes cxl_sysram_region >>> devices and creates cxl_dax_region with dax_driver=kmem. >>> >>> This creates an interposition step where users can configure policy. >>> >>> Device hierarchy: >>> region0 -> sysram_region0 -> dax_region0 -> dax0.0 >> >> This technically comes up in the devdax_region driver patch first, but I >> noticed it here >> so this is where I'm putting it: >> >> I like the idea here, but the implementation is all off. Firstly, >> devm_cxl_add_sysram_region() >> is never called outside of sysram_region_driver::probe(), so I'm not sure >> how they ever get >> added to the system (same with devdax regions). >> >> Second, there's this weird pattern of adding sub-region (sysram, devdax, >> etc.) devices being added >> inside of the sub-region driver probe. I would expect the devices are added >> then the probe function >> is called. > > I originally tried doing with region0/region_driver, but that design > pattern is also confusing - and it creates differently bad patterns. > > echo region0 > decoder0.0/create_ram_region -> creates region0 > > # Current pattern > echo region > driver/region/probe /* auto-region behavior */ > > # region_driver attribute pattern > echo "sysram" > region0/region_driver > echo region0 > driver/region/probe /* uses sysram region driver */ > > https://lore.kernel.org/linux-cxl/[email protected]/ > > Ira pointed out that this design makes the "implicit" design of the > driver worse. The user doesn't actually know what driver is being used > under the hood - it just knows something is being used. > > This at least makes it explicit which driver is being used - and splits > the uses-case logic up into discrete drivers (dax users don't have to > worry about sysram users breaking their stuff). > > If it makes more sense, you could swap the ordering of the names > > echo region0 > region/bind > echo region0 > region_sysram/bind > echo region0 > region_daxdev/bind > echo region0 > region_dax_kmem/bind > echo region0 > region_pony/bind > > --- > > The underlying issue is that region::probe() is trying to be a > god-function for every possible use case, and hiding the use case > behind an attribute vs a driver is not good. > > (also the default behavior for region::probe() in an otherwise > unconfigured region is required for backwards compatibility)
Ok, that makes sense. I think I just got lost in the sauce while looking at this last week and this explanation helped a lot.> >> What I think should be going on here (and correct me if I'm wrong) is: >> 1. a cxl_region device is added to the system >> 2. cxl_region::probe() is called on said device (one in >> cxl/core/region.c) >> 3. Said probe function figures out the device is a dax_region or >> whatever else and creates that type of region device >> (i.e. cxl_region::probe() -> device_add(&cxl_sysram_device)) >> 4. if the device's dax driver type is DAXDRV_DEVICE_TYPE it gets sent >> to the daxdev_region driver >> 5a. if the device's dax driver type is DAXDRV_KMEM_TYPE it gets sent to >> the sysram_region driver which holds it until >> the online_type is set >> 5b. Once the online_type is set, the device is forwarded to the >> dax_kmem_region driver? Not sure on this part >> >> What seems to be happening is that the cxl_region is added, all of these >> region drivers try >> to bind to it since they all use the same device id (CXL_DEVICE_REGION) and >> the correct one is >> figured out by magic? I'm somewhat confused at this point :/. >> > > For auto-regions: > region_probe() eats it and you get the default behavior. > > For non-auto regions: > create_x_region generates an un-configured region and fails to probe > until the user commits it and probes it. I think this was the source of my misunderstanding. I was trying to understand how it works for auto regions when it's never meant to apply to them. Sorry if this is a stupid question, but what stops auto regions from binding to the sysram/dax region drivers? They all bind to region devices, so I assume there's something keeping them from binding before the core region driver gets a chance. Thanks, Ben > > auto-regions are evil and should be discouraged. > > ~Gregory
