On Wed, Apr 19, 2017 at 11:19 AM, Logan Gunthorpe <logang@xxxxxxxxxxxx> wrote: > > > On 19/04/17 12:11 PM, Logan Gunthorpe wrote: >> >> >> On 19/04/17 11:41 AM, Dan Williams wrote: >>> No, not quite ;-). I still don't think we should require the non-HMM >>> to pass NULL for all the HMM arguments. What I like about Logan's >>> proposal is to have a separate create and register steps dev_pagemap. >>> That way call paths that don't care about HMM specifics can just turn >>> around and register the vanilla dev_pagemap. >> >> Would you necessarily even need a create step? I was thinking more along >> the lines that struct dev_pagemap _could_ just be a member in another >> structure. The caller would set the attributes they needed and pass it >> to devm_memremap. (Similar to how we commonly do things with struct >> device, et al). Potentially, that could also get rid of the need for the >> *data pointer HMM is using to get back the struct hmm_devmem seeing >> container_of could be used instead. > > Also, now that I've thought about it a little more, it _may_ be that > many or all of the hmm specific fields in dev_pagemap could move to a > containing struct too... Yes, that's already how we handle struct page_map, it's an internal implementation detail of devm_memremap_pages(). Letting others users do the container_of() arrangement means that struct page_map needs to become public and move into struct dev_pagemap directly. ...I think that encapsulation loss is worth it for the gain of clearly separating the HMM-case from the base case.
