On Fri, Sep 14, 2018 at 6:16 AM, Christoph Hellwig <hch@xxxxxx> wrote: >> An argument could be made to require that the ->kill() operation be set >> in the @pgmap arg rather than passed in separately. However, it helps >> code readability, tracking the lifetime of a given instance, to be able >> to grep the kill routine directly at the devm_memremap_pages() call >> site. > > I generally do not like passing redundant argument, and I don't really > see why this case is different. Or in other ways I'd like to make > your above argument.. Logan had similar feedback, and now the chorus is getting louder. I personally like how I can do this with grep: drivers/dax/pmem.c:114: addr = devm_memremap_pages(dev, &dax_pmem->pgmap, dax_pmem_percpu_kill); -- drivers/nvdimm/pmem.c:411: addr = devm_memremap_pages(dev, &pmem->pgmap, drivers/nvdimm/pmem.c-412- pmem_freeze_queue); -- drivers/nvdimm/pmem.c:425: addr = devm_memremap_pages(dev, &pmem->pgmap, drivers/nvdimm/pmem.c-426- pmem_freeze_queue); -- mm/hmm.c:1059: result = devm_memremap_pages(devmem->device, &devmem->pagemap, mm/hmm.c-1060- hmm_devmem_ref_kill); -- mm/hmm.c:1113: result = devm_memremap_pages(devmem->device, &devmem->pagemap, mm/hmm.c-1114- hmm_devmem_ref_kill); ...and see all of the kill() variants, but the redundancy will likely continue to bother folks. > Except for that the patch looks good to me. Thanks, I'll fix it up to drop the redundant arg.
