From: Ross Zwisler <ross.zwisler@xxxxxxxxxxxxxxx> Sent: Tuesday, July 24, 2018 10:50 PM > > Some functions within fs/dax and dax/super don't need to get kaddr from > > direct_access. Assigning NULL to kaddr to ->direct_access() is more > > straightforward and simple than offering a useless local pointer. > > > > So all direct_access() need to check the validity of second rank pointer > > kaddr for NULL assignment. If kaddr equals to NULL, it doesn't need to > > calculate its value. > > > > * This series are supplement to [PATCH v2 00/14]mm: Asynchronous + > > multithreaded memmap init for ZONE_DEVICE. [1] > > > > [1]: https://lkml.org/lkml/2018/7/16/828 > > This whole series looks good to me. Just a few comments: > > 1) Does this series actually depend on the "Asynchronous multithreaded mmap > init for ZONE_DEVICE" series from Dan? It seems totally independent to me? > I reviewed yours by applying to linux/master, which worked fine. I ask > because Dan's series has been delayed to after v4.19, and if yours isn't > actually dependent it could possibly go in sooner. This series doesn't depend on Dan's 'Asynchronous multithreaded mmap init for ZONE_DEVICE'. For the part as pfn, which overlaps Dan's original series. Because I post them earlier than Dan's, Dan generously dropped the overlapping from his series and adopted mine to the series of 'Asynchronous multithreaded'. It is very thankful. I knew Dan's series would be delayed, I can resend the series, both kaddr and pfn, for faster merging to mainline. > 2) I agree with Christian's comment that the changelogs could be improved > slightly. Remember that the goal of the changelog isn't to describe *what* > the code is doing, but *why*. We can read that the code now checks if 'kaddr' > is NULL, and if so we don't calculate it. It's useful to say that callers may > have no need for 'kaddr', so this patch is prep for allowing them to pass in > NULL instead of having to pass in a pointer that they then just throw away. Thanks for advice. I will follow your suggestion during next submission. > 3) I think you should make one more change to kill the unused 'dummy_addr' > variable in persistent_memory_claim(). That was the one last case of a dummy > 'kaddr' type variable that I could find. Yes, you are right. I haven't updated my code base to latest mainline, so couldn't notice it. It seems there is a new created file here as dm-writecache.c. Thanks for hints. Cheers, Huaisheng Ye
