| Message ID | 20200321003108.22941-1-rcampbell@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm/hmm/test: add self tests for HMM | expand |
On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > This series adds basic self tests for HMM and are intended for Jason > Gunthorpe's rdma tree which has a number of HMM patches applied. > > Changes v7 -> v8: > Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series > "Small hmm_range_fault() cleanups". > Applied a number of changes from Jason's comments. > > Changes v6 -> v7: > Rebased to linux-5.6.0-rc6 > Reverted back to just using mmu_interval_notifier_insert() and making > this series only introduce HMM self tests. > > Changes v5 -> v6: > Rebased to linux-5.5.0-rc6 > Refactored mmu interval notifier patches > Converted nouveau to use the new mmu interval notifier API > > Changes v4 -> v5: > Added mmu interval notifier insert/remove/update callable from the > invalidate() callback > Updated HMM tests to use the new core interval notifier API > > Changes v1 -> v4: > https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com > > Ralph Campbell (3): > mm/hmm/test: add selftest driver for HMM > mm/hmm/test: add selftests for HMM > MAINTAINERS: add HMM selftests > > MAINTAINERS | 3 + > include/uapi/linux/test_hmm.h | 59 ++ Isn't UAPI folder supposed to be for user-visible interfaces that follow the rule of non-breaking user space and not for selftests? Thanks
On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > This series adds basic self tests for HMM and are intended for Jason > Gunthorpe's rdma tree which has a number of HMM patches applied. We are at v8 of this series and noboy from the selftests land has commented, can someone help? Jason
On 3/21/20 2:00 AM, Leon Romanovsky wrote: > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: >> This series adds basic self tests for HMM and are intended for Jason >> Gunthorpe's rdma tree which has a number of HMM patches applied. >> >> Changes v7 -> v8: >> Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series >> "Small hmm_range_fault() cleanups". >> Applied a number of changes from Jason's comments. >> >> Changes v6 -> v7: >> Rebased to linux-5.6.0-rc6 >> Reverted back to just using mmu_interval_notifier_insert() and making >> this series only introduce HMM self tests. >> >> Changes v5 -> v6: >> Rebased to linux-5.5.0-rc6 >> Refactored mmu interval notifier patches >> Converted nouveau to use the new mmu interval notifier API >> >> Changes v4 -> v5: >> Added mmu interval notifier insert/remove/update callable from the >> invalidate() callback >> Updated HMM tests to use the new core interval notifier API >> >> Changes v1 -> v4: >> https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com >> >> Ralph Campbell (3): >> mm/hmm/test: add selftest driver for HMM >> mm/hmm/test: add selftests for HMM >> MAINTAINERS: add HMM selftests >> >> MAINTAINERS | 3 + >> include/uapi/linux/test_hmm.h | 59 ++ > > Isn't UAPI folder supposed to be for user-visible interfaces that follow > the rule of non-breaking user space and not for selftests? > > Thanks > Most of the other kernel module tests seem to invoke the test as part of the module load/init. I'm open to moving it if there is a more appropriate location.
On Sat, Mar 21, 2020 at 10:27:46AM -0700, Ralph Campbell wrote: > > On 3/21/20 2:00 AM, Leon Romanovsky wrote: > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > This series adds basic self tests for HMM and are intended for Jason > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > > > Changes v7 -> v8: > > > Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series > > > "Small hmm_range_fault() cleanups". > > > Applied a number of changes from Jason's comments. > > > > > > Changes v6 -> v7: > > > Rebased to linux-5.6.0-rc6 > > > Reverted back to just using mmu_interval_notifier_insert() and making > > > this series only introduce HMM self tests. > > > > > > Changes v5 -> v6: > > > Rebased to linux-5.5.0-rc6 > > > Refactored mmu interval notifier patches > > > Converted nouveau to use the new mmu interval notifier API > > > > > > Changes v4 -> v5: > > > Added mmu interval notifier insert/remove/update callable from the > > > invalidate() callback > > > Updated HMM tests to use the new core interval notifier API > > > > > > Changes v1 -> v4: > > > https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com > > > > > > Ralph Campbell (3): > > > mm/hmm/test: add selftest driver for HMM > > > mm/hmm/test: add selftests for HMM > > > MAINTAINERS: add HMM selftests > > > > > > MAINTAINERS | 3 + > > > include/uapi/linux/test_hmm.h | 59 ++ > > > > Isn't UAPI folder supposed to be for user-visible interfaces that follow > > the rule of non-breaking user space and not for selftests? > > > > Thanks > > > > Most of the other kernel module tests seem to invoke the test as part of the > module load/init. I'm open to moving it if there is a more appropriate location. Is it even possible to create a user mm_struct and put crazy things in it soley from a kernel module? Jason
On Sat, Mar 21, 2020 at 06:55:05PM -0300, Jason Gunthorpe wrote: > On Sat, Mar 21, 2020 at 10:27:46AM -0700, Ralph Campbell wrote: > > > > On 3/21/20 2:00 AM, Leon Romanovsky wrote: > > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > > This series adds basic self tests for HMM and are intended for Jason > > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > > > > > Changes v7 -> v8: > > > > Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series > > > > "Small hmm_range_fault() cleanups". > > > > Applied a number of changes from Jason's comments. > > > > > > > > Changes v6 -> v7: > > > > Rebased to linux-5.6.0-rc6 > > > > Reverted back to just using mmu_interval_notifier_insert() and making > > > > this series only introduce HMM self tests. > > > > > > > > Changes v5 -> v6: > > > > Rebased to linux-5.5.0-rc6 > > > > Refactored mmu interval notifier patches > > > > Converted nouveau to use the new mmu interval notifier API > > > > > > > > Changes v4 -> v5: > > > > Added mmu interval notifier insert/remove/update callable from the > > > > invalidate() callback > > > > Updated HMM tests to use the new core interval notifier API > > > > > > > > Changes v1 -> v4: > > > > https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com > > > > > > > > Ralph Campbell (3): > > > > mm/hmm/test: add selftest driver for HMM > > > > mm/hmm/test: add selftests for HMM > > > > MAINTAINERS: add HMM selftests > > > > > > > > MAINTAINERS | 3 + > > > > include/uapi/linux/test_hmm.h | 59 ++ > > > > > > Isn't UAPI folder supposed to be for user-visible interfaces that follow > > > the rule of non-breaking user space and not for selftests? > > > > > > Thanks > > > > > > > Most of the other kernel module tests seem to invoke the test as part of the > > module load/init. I'm open to moving it if there is a more appropriate location. > > Is it even possible to create a user mm_struct and put crazy things in > it soley from a kernel module? I didn't look very closely of what Ralph did in his patchsets, but from what I know, if you want in-kernel interface, you use in-kernel module, if you want to test user visible uapi, you write application. You don't create new UAPI just to test something in the kernel. Can kunit help here? https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html Thanks > > Jason >
On 3/22/20 1:10 AM, Leon Romanovsky wrote: > On Sat, Mar 21, 2020 at 06:55:05PM -0300, Jason Gunthorpe wrote: >> On Sat, Mar 21, 2020 at 10:27:46AM -0700, Ralph Campbell wrote: >>> >>> On 3/21/20 2:00 AM, Leon Romanovsky wrote: >>>> On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: >>>>> This series adds basic self tests for HMM and are intended for Jason >>>>> Gunthorpe's rdma tree which has a number of HMM patches applied. >>>>> >>>>> Changes v7 -> v8: >>>>> Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series >>>>> "Small hmm_range_fault() cleanups". >>>>> Applied a number of changes from Jason's comments. >>>>> >>>>> Changes v6 -> v7: >>>>> Rebased to linux-5.6.0-rc6 >>>>> Reverted back to just using mmu_interval_notifier_insert() and making >>>>> this series only introduce HMM self tests. >>>>> >>>>> Changes v5 -> v6: >>>>> Rebased to linux-5.5.0-rc6 >>>>> Refactored mmu interval notifier patches >>>>> Converted nouveau to use the new mmu interval notifier API >>>>> >>>>> Changes v4 -> v5: >>>>> Added mmu interval notifier insert/remove/update callable from the >>>>> invalidate() callback >>>>> Updated HMM tests to use the new core interval notifier API >>>>> >>>>> Changes v1 -> v4: >>>>> https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com >>>>> >>>>> Ralph Campbell (3): >>>>> mm/hmm/test: add selftest driver for HMM >>>>> mm/hmm/test: add selftests for HMM >>>>> MAINTAINERS: add HMM selftests >>>>> >>>>> MAINTAINERS | 3 + >>>>> include/uapi/linux/test_hmm.h | 59 ++ >>>> >>>> Isn't UAPI folder supposed to be for user-visible interfaces that follow >>>> the rule of non-breaking user space and not for selftests? >>>> >>>> Thanks >>>> >>> >>> Most of the other kernel module tests seem to invoke the test as part of the >>> module load/init. I'm open to moving it if there is a more appropriate location. >> >> Is it even possible to create a user mm_struct and put crazy things in >> it soley from a kernel module? > > I didn't look very closely of what Ralph did in his patchsets, but from > what I know, if you want in-kernel interface, you use in-kernel module, > if you want to test user visible uapi, you write application. You don't > create new UAPI just to test something in the kernel. > > Can kunit help here? > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html > > Thanks > >> >> Jason The tests are intended to cover hmm_range_fault() and the migrate_vma_setup(), migrate_vma_pages(), and migrate_vma_finalize() kernel functions that a device driver can call to initialize hardware that has its own page tables. An example is a GPU where the code on the GPU sees the same address space as code running on the host CPU. This means the test has to have a user process to create a user process address space and a device driver to simulate some real device driver. The UAPI is for the user level test program to tell the kernel module test driver what to do and return results. The complexity is all around maintaining coherent copies of the user process page tables while hardware and CPUs are accessing the same physical addresses. The pages are not pinned as with most I/O so system activity like pagein/pageout, LRU page reclaim, compaction, and the process calling functions like mmap(), mprotect(), madvise(), fork(), etc. all update the CPU and device page tables and would be very hard to duplicate in a kernel level only KUNIT style of test.
On Sun, Mar 22, 2020 at 10:10:38AM +0200, Leon Romanovsky wrote: > On Sat, Mar 21, 2020 at 06:55:05PM -0300, Jason Gunthorpe wrote: > > On Sat, Mar 21, 2020 at 10:27:46AM -0700, Ralph Campbell wrote: > > > > > > On 3/21/20 2:00 AM, Leon Romanovsky wrote: > > > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > > > This series adds basic self tests for HMM and are intended for Jason > > > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > > > > > > > Changes v7 -> v8: > > > > > Rebased to Jason's rdma/hmm tree, plus Jason's 6 patch series > > > > > "Small hmm_range_fault() cleanups". > > > > > Applied a number of changes from Jason's comments. > > > > > > > > > > Changes v6 -> v7: > > > > > Rebased to linux-5.6.0-rc6 > > > > > Reverted back to just using mmu_interval_notifier_insert() and making > > > > > this series only introduce HMM self tests. > > > > > > > > > > Changes v5 -> v6: > > > > > Rebased to linux-5.5.0-rc6 > > > > > Refactored mmu interval notifier patches > > > > > Converted nouveau to use the new mmu interval notifier API > > > > > > > > > > Changes v4 -> v5: > > > > > Added mmu interval notifier insert/remove/update callable from the > > > > > invalidate() callback > > > > > Updated HMM tests to use the new core interval notifier API > > > > > > > > > > Changes v1 -> v4: > > > > > https://lore.kernel.org/linux-mm/20191104222141.5173-1-rcampbell@nvidia.com > > > > > > > > > > Ralph Campbell (3): > > > > > mm/hmm/test: add selftest driver for HMM > > > > > mm/hmm/test: add selftests for HMM > > > > > MAINTAINERS: add HMM selftests > > > > > > > > > > MAINTAINERS | 3 + > > > > > include/uapi/linux/test_hmm.h | 59 ++ > > > > > > > > Isn't UAPI folder supposed to be for user-visible interfaces that follow > > > > the rule of non-breaking user space and not for selftests? > > > > > > > > Thanks > > > > > > > > > > Most of the other kernel module tests seem to invoke the test as part of the > > > module load/init. I'm open to moving it if there is a more appropriate location. > > > > Is it even possible to create a user mm_struct and put crazy things in > > it soley from a kernel module? > > I didn't look very closely of what Ralph did in his patchsets, but from > what I know, if you want in-kernel interface, you use in-kernel module, > if you want to test user visible uapi, you write application. You don't > create new UAPI just to test something in the kernel. That works fine as long as the in-kernel interfaces don't require user created objects like mm_struct and vmas, which is the case here. So there must be some special uAPI between the kerne/user to make it work. Jason
On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > This series adds basic self tests for HMM and are intended for Jason > Gunthorpe's rdma tree which has a number of HMM patches applied. Here are some hunks I noticed while testing this: --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2201,7 +2201,8 @@ config TEST_MEMINIT config TEST_HMM tristate "Test HMM (Heterogeneous Memory Management)" - depends on DEVICE_PRIVATE + depends on TRANSPARENT_HUGEPAGE + select DEVICE_PRIVATE select HMM_MIRROR select MMU_NOTIFIER help It fails testing if TRANSPARENT_HUGEPAGE is not on @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) spin_lock_init(&mdevice->lock); cdev_init(&mdevice->cdevice, &dmirror_fops); + mdevice->cdevice.owner = THIS_MODULE; ret = cdev_add(&mdevice->cdevice, dev, 1); if (ret) return ret; The use of cdev without a struct device is super weird, but it still needs this diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh index 461e4a99a362cf..0647b525a62564 100755 --- a/tools/testing/selftests/vm/test_hmm.sh +++ b/tools/testing/selftests/vm/test_hmm.sh @@ -59,7 +59,7 @@ run_smoke() echo "Running smoke test. Note, this test provides basic coverage." load_driver - ./hmm-tests + $(dirname "${BASH_SOURCE[0]}")/hmm-tests unload_driver } Make it runnably reliably Jason
On 4/15/20 7:41 AM, Jason Gunthorpe wrote: > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: >> This series adds basic self tests for HMM and are intended for Jason >> Gunthorpe's rdma tree which has a number of HMM patches applied. > > Here are some hunks I noticed while testing this: > > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2201,7 +2201,8 @@ config TEST_MEMINIT > > config TEST_HMM > tristate "Test HMM (Heterogeneous Memory Management)" > - depends on DEVICE_PRIVATE > + depends on TRANSPARENT_HUGEPAGE > + select DEVICE_PRIVATE > select HMM_MIRROR > select MMU_NOTIFIER > help > > It fails testing if TRANSPARENT_HUGEPAGE is not on > > @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) > spin_lock_init(&mdevice->lock); > > cdev_init(&mdevice->cdevice, &dmirror_fops); > + mdevice->cdevice.owner = THIS_MODULE; > ret = cdev_add(&mdevice->cdevice, dev, 1); > if (ret) > return ret; > > The use of cdev without a struct device is super weird, but it still > needs this > > diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh > index 461e4a99a362cf..0647b525a62564 100755 > --- a/tools/testing/selftests/vm/test_hmm.sh > +++ b/tools/testing/selftests/vm/test_hmm.sh > @@ -59,7 +59,7 @@ run_smoke() > echo "Running smoke test. Note, this test provides basic coverage." > > load_driver > - ./hmm-tests > + $(dirname "${BASH_SOURCE[0]}")/hmm-tests > unload_driver > } > > Make it runnably reliably > > Jason Thanks for the fixes. I'll apply these and send a v9. I will also add missing calls to release_mem_region() to free the reserved device private addresses.
On Wed, Apr 15, 2020 at 10:28:23AM -0700, Ralph Campbell wrote: > > On 4/15/20 7:41 AM, Jason Gunthorpe wrote: > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > This series adds basic self tests for HMM and are intended for Jason > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > Here are some hunks I noticed while testing this: > > > > --- a/lib/Kconfig.debug > > +++ b/lib/Kconfig.debug > > @@ -2201,7 +2201,8 @@ config TEST_MEMINIT > > config TEST_HMM > > tristate "Test HMM (Heterogeneous Memory Management)" > > - depends on DEVICE_PRIVATE > > + depends on TRANSPARENT_HUGEPAGE > > + select DEVICE_PRIVATE > > select HMM_MIRROR > > select MMU_NOTIFIER > > help > > > > It fails testing if TRANSPARENT_HUGEPAGE is not on > > > > @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) > > spin_lock_init(&mdevice->lock); > > cdev_init(&mdevice->cdevice, &dmirror_fops); > > + mdevice->cdevice.owner = THIS_MODULE; > > ret = cdev_add(&mdevice->cdevice, dev, 1); > > if (ret) > > return ret; > > > > The use of cdev without a struct device is super weird, but it still > > needs this > > > > diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh > > index 461e4a99a362cf..0647b525a62564 100755 > > --- a/tools/testing/selftests/vm/test_hmm.sh > > +++ b/tools/testing/selftests/vm/test_hmm.sh > > @@ -59,7 +59,7 @@ run_smoke() > > echo "Running smoke test. Note, this test provides basic coverage." > > load_driver > > - ./hmm-tests > > + $(dirname "${BASH_SOURCE[0]}")/hmm-tests > > unload_driver > > } > > > > Make it runnably reliably > > > > Jason > > Thanks for the fixes. I'll apply these and send a v9. > I will also add missing calls to release_mem_region() to free the reserved device private > addresses. If you decide to ignore my request to avoid addition of special header file to UAPI, at least don't copy and install that file without some special CONFIG option (TEST_HMM ???) requested by the users. It also will be good to get Acked-by on this change from HMM people. However, I still think that include/uapi/linux/test_hmm.h opens pandora box of having UAPI files without real promise to keep it backward compatible. Thanks >
On Wed, Apr 15, 2020 at 10:29:52PM +0300, Leon Romanovsky wrote: > On Wed, Apr 15, 2020 at 10:28:23AM -0700, Ralph Campbell wrote: > > > > On 4/15/20 7:41 AM, Jason Gunthorpe wrote: > > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > > This series adds basic self tests for HMM and are intended for Jason > > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > > > Here are some hunks I noticed while testing this: > > > > > > +++ b/lib/Kconfig.debug > > > @@ -2201,7 +2201,8 @@ config TEST_MEMINIT > > > config TEST_HMM > > > tristate "Test HMM (Heterogeneous Memory Management)" > > > - depends on DEVICE_PRIVATE > > > + depends on TRANSPARENT_HUGEPAGE > > > + select DEVICE_PRIVATE > > > select HMM_MIRROR > > > select MMU_NOTIFIER > > > help > > > > > > It fails testing if TRANSPARENT_HUGEPAGE is not on > > > > > > @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) > > > spin_lock_init(&mdevice->lock); > > > cdev_init(&mdevice->cdevice, &dmirror_fops); > > > + mdevice->cdevice.owner = THIS_MODULE; > > > ret = cdev_add(&mdevice->cdevice, dev, 1); > > > if (ret) > > > return ret; > > > > > > The use of cdev without a struct device is super weird, but it still > > > needs this > > > > > > diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh > > > index 461e4a99a362cf..0647b525a62564 100755 > > > +++ b/tools/testing/selftests/vm/test_hmm.sh > > > @@ -59,7 +59,7 @@ run_smoke() > > > echo "Running smoke test. Note, this test provides basic coverage." > > > load_driver > > > - ./hmm-tests > > > + $(dirname "${BASH_SOURCE[0]}")/hmm-tests > > > unload_driver > > > } > > > > > > Make it runnably reliably > > > > > > Jason > > > > Thanks for the fixes. I'll apply these and send a v9. > > I will also add missing calls to release_mem_region() to free the reserved device private > > addresses. > > If you decide to ignore my request to avoid addition of special header > file to UAPI, at least don't copy and install that file without some > special CONFIG option (TEST_HMM ???) requested by the users. It also > will be good to get Acked-by on this change from HMM people. > > However, I still think that include/uapi/linux/test_hmm.h opens > pandora box of having UAPI files without real promise to keep it > backward compatible. It would be nice if we could put the header outside the uapi directory and outside the install machinery Maybe for now hackery some relative include like #include "../../../lib/hmm_test_uapi.h" ? I don't see any sane way to avoid the dedicate module and special ioctl though. Jason
On 4/15/20 12:29 PM, Leon Romanovsky wrote: > On Wed, Apr 15, 2020 at 10:28:23AM -0700, Ralph Campbell wrote: >> >> On 4/15/20 7:41 AM, Jason Gunthorpe wrote: >>> On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: >>>> This series adds basic self tests for HMM and are intended for Jason >>>> Gunthorpe's rdma tree which has a number of HMM patches applied. >>> >>> Here are some hunks I noticed while testing this: >>> >>> --- a/lib/Kconfig.debug >>> +++ b/lib/Kconfig.debug >>> @@ -2201,7 +2201,8 @@ config TEST_MEMINIT >>> config TEST_HMM >>> tristate "Test HMM (Heterogeneous Memory Management)" >>> - depends on DEVICE_PRIVATE >>> + depends on TRANSPARENT_HUGEPAGE >>> + select DEVICE_PRIVATE >>> select HMM_MIRROR >>> select MMU_NOTIFIER >>> help >>> >>> It fails testing if TRANSPARENT_HUGEPAGE is not on >>> >>> @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) >>> spin_lock_init(&mdevice->lock); >>> cdev_init(&mdevice->cdevice, &dmirror_fops); >>> + mdevice->cdevice.owner = THIS_MODULE; >>> ret = cdev_add(&mdevice->cdevice, dev, 1); >>> if (ret) >>> return ret; >>> >>> The use of cdev without a struct device is super weird, but it still >>> needs this >>> >>> diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh >>> index 461e4a99a362cf..0647b525a62564 100755 >>> --- a/tools/testing/selftests/vm/test_hmm.sh >>> +++ b/tools/testing/selftests/vm/test_hmm.sh >>> @@ -59,7 +59,7 @@ run_smoke() >>> echo "Running smoke test. Note, this test provides basic coverage." >>> load_driver >>> - ./hmm-tests >>> + $(dirname "${BASH_SOURCE[0]}")/hmm-tests >>> unload_driver >>> } >>> >>> Make it runnably reliably >>> >>> Jason >> >> Thanks for the fixes. I'll apply these and send a v9. >> I will also add missing calls to release_mem_region() to free the reserved device private >> addresses. > > If you decide to ignore my request to avoid addition of special header > file to UAPI, at least don't copy and install that file without some > special CONFIG option (TEST_HMM ???) requested by the users. It also > will be good to get Acked-by on this change from HMM people. > > However, I still think that include/uapi/linux/test_hmm.h opens > pandora box of having UAPI files without real promise to keep it > backward compatible. > > Thanks I think that is a valid point. I would expect the test<->driver UAPI to track the kernel version since the sources are "released" together. I suppose a version number could be included in the request structure to handle mismatch driver and test program but that may be overkill. Are you suggesting that include/linux/test_hmm.h is a better location?
On Wed, Apr 15, 2020 at 12:39:45PM -0700, Ralph Campbell wrote: > > On 4/15/20 12:29 PM, Leon Romanovsky wrote: > > On Wed, Apr 15, 2020 at 10:28:23AM -0700, Ralph Campbell wrote: > > > > > > On 4/15/20 7:41 AM, Jason Gunthorpe wrote: > > > > On Fri, Mar 20, 2020 at 05:31:05PM -0700, Ralph Campbell wrote: > > > > > This series adds basic self tests for HMM and are intended for Jason > > > > > Gunthorpe's rdma tree which has a number of HMM patches applied. > > > > > > > > Here are some hunks I noticed while testing this: > > > > > > > > --- a/lib/Kconfig.debug > > > > +++ b/lib/Kconfig.debug > > > > @@ -2201,7 +2201,8 @@ config TEST_MEMINIT > > > > config TEST_HMM > > > > tristate "Test HMM (Heterogeneous Memory Management)" > > > > - depends on DEVICE_PRIVATE > > > > + depends on TRANSPARENT_HUGEPAGE > > > > + select DEVICE_PRIVATE > > > > select HMM_MIRROR > > > > select MMU_NOTIFIER > > > > help > > > > > > > > It fails testing if TRANSPARENT_HUGEPAGE is not on > > > > > > > > @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) > > > > spin_lock_init(&mdevice->lock); > > > > cdev_init(&mdevice->cdevice, &dmirror_fops); > > > > + mdevice->cdevice.owner = THIS_MODULE; > > > > ret = cdev_add(&mdevice->cdevice, dev, 1); > > > > if (ret) > > > > return ret; > > > > > > > > The use of cdev without a struct device is super weird, but it still > > > > needs this > > > > > > > > diff --git a/tools/testing/selftests/vm/test_hmm.sh b/tools/testing/selftests/vm/test_hmm.sh > > > > index 461e4a99a362cf..0647b525a62564 100755 > > > > --- a/tools/testing/selftests/vm/test_hmm.sh > > > > +++ b/tools/testing/selftests/vm/test_hmm.sh > > > > @@ -59,7 +59,7 @@ run_smoke() > > > > echo "Running smoke test. Note, this test provides basic coverage." > > > > load_driver > > > > - ./hmm-tests > > > > + $(dirname "${BASH_SOURCE[0]}")/hmm-tests > > > > unload_driver > > > > } > > > > > > > > Make it runnably reliably > > > > > > > > Jason > > > > > > Thanks for the fixes. I'll apply these and send a v9. > > > I will also add missing calls to release_mem_region() to free the reserved device private > > > addresses. > > > > If you decide to ignore my request to avoid addition of special header > > file to UAPI, at least don't copy and install that file without some > > special CONFIG option (TEST_HMM ???) requested by the users. It also > > will be good to get Acked-by on this change from HMM people. > > > > However, I still think that include/uapi/linux/test_hmm.h opens > > pandora box of having UAPI files without real promise to keep it > > backward compatible. > > > > Thanks > > I think that is a valid point. I would expect the test<->driver UAPI to track the kernel > version since the sources are "released" together. I suppose a version number could be > included in the request structure to handle mismatch driver and test program but that > may be overkill. Yes, it is really overkill. > Are you suggesting that include/linux/test_hmm.h is a better location? It is one of options, another option maybe similar to that is done in scripts/mod/modpost.c [1], where C file is generated on the fly. [1] https://lore.kernel.org/netdev/20200415133648.1306956-5-leon@kernel.org