HWPMC tracing support (2) -- VM

[kib]

sys/dev/hwpmc/hwpmc_vm.c

76

Did you looked at OSD, sys/osd.h ? OSD_THREAD should remove the ugly side-list of threads.

79

What protects the map->obj from the parallel dereference ?

103

This should be CPU_FOREACH(). Or instead, just allocate the whole array by one malloc().

110

You should use make_dev_s(), to guard againts si_drv1 races.

[br]

Use:

1. osd(9)
2. CPU_FOREACH macro
3. make_dev_s instead of make_dev
4. mtx_lock around map->obj dereferencing

[kib]

sys/dev/hwpmc/hwpmc_vm.c

79

I still do not see any protection against possible object deallocation.

147

Why destroy_dev_sched() and not destroy_dev() ?

[br]

• reference the object
• use destroy_dev, not destroy_dev_sched

[br]

sys/dev/hwpmc/hwpmc_vm.c

79

Added vm_object_reference() call

147

No reason, just testing. Restored

[kib]

Ok.

Where is the dereference of the vm object reference owned by the OSD ?

[br]

In D15089#318912, @kib wrote:

Ok.

Where is the dereference of the vm object reference owned by the OSD ?

1. in pt_buffer_deallocate() @ https://reviews.freebsd.org/D15091
2. in coresight_buffer_deallocate() @ https://reviews.freebsd.org/D15090

[br]

Regenerate patch

[br]

merge changes from upstream

[br]

regenerate

[br]

regenerate

[br]

regenerate

[markj]

sys/dev/hwpmc/hwpmc_vm.c

122

Why is the device writable? Isn't the purpose just to provide read-only trace buffers?

145

Isn't osd_thread_deregister() needed?

[br]

• deregister thread OSD
• use read-only permissions for /dev/pmc*

[br]

sys/dev/hwpmc/hwpmc_vm.c

122

Yes. Thanks. I changed to read-only.

145

Yes, it is needed. I missed this. Thanks!

[markj]

sys/dev/hwpmc/hwpmc_vm.c

145

I think the appropriate order here would be to destroy the device first.

[br]

destroy the character device first, and then osd_thread_deregister()

[markj]

sys/dev/hwpmc/hwpmc_vm.c

128

I think osd_thread_deregister() needs to be called in this path too.

sys/modules/hwpmc/Makefile

13

Seems this is unrelated to the rest of the diff.

18

Ditto.

[br]

deregister osd thread on failure

[br]

correctly deregister osd thread on failure

[br]

• fix destroying device on failure
• destroy mtx lock

[br]

fix memory leak on failure

[br]

another attempt to handle failure case correctly

[br]

sys/modules/hwpmc/Makefile

13

these two chunks are not related to VM, but I am lazy to edit the patch every time I submit changes.
All parts will go together anyway

[br]

rebase

[br]

Regenerate patch.