Fix a couple of issues in nm_os_extmem_create

[markj]

sys/dev/netmap/netmap_freebsd.c

740

If the processes mmapping it exit, then the /dev/netmap handle will be closed and the destructor will remove this mapping anyway. It looks like the real issue is that processes may munmap() the registered mapping?

768

I think this should really come after each failed vm_map_* call, before the goto.

[vmaffione]

Addressed reviewer comments

[vmaffione]

sys/dev/netmap/netmap_freebsd.c

740

If I understand correctly you are only suggesting a comment fix?

768

It should be equivalent, because all the gotos (except for the first) would pass here.
In any case, it's more readable as you suggest.

[markj]

Code changes seem ok to me, thank you.

sys/dev/netmap/netmap_freebsd.c

740

Well, I believe the comment is wrong, but maybe I misunderstood. Like, is this kernel mapping ever actually accessed by netmap?

768

Right, this is just in case some code is added later which does not use Mach return codes.

[vmaffione]

Il giorno ven 19 mar 2021 alle ore 18:24 markj (Mark Johnston) <
[e-mail]> ha scritto:

markj accepted this revision.
markj added a comment.
This revision is now accepted and ready to land.

Code changes seem ok to me, thank you.

INLINE COMMENTS

vmaffione wrote in netmap_freebsd.c:740
If I understand correctly you are only suggesting a comment fix?

Well, I believe the comment is wrong, but maybe I misunderstood. Like, is
this kernel mapping ever actually accessed by netmap?

It looks incorrect also to me. I think the kernel mapping is accessed by
netmap (kernel) at least to touch the netmap_ring structs, and initialize
netmap_if structs (in some cases, VALE can also touch the netmap buffers).
But I may be misunderstanding, since "extmem" is a kind of recent addition.
Maybe Giuseppe can shed a light on this matter.

vmaffione wrote in netmap_freebsd.c:768
It should be equivalent, because all the gotos (except for the first)

would pass here.

In any case, it's more readable as you suggest.

Right, this is just in case some code is added later which does not use
Mach return codes.

Sounds good, thanks.

CHANGES SINCE LAST ACTION

https://reviews.freebsd.org/D29318/new/

REVISION DETAIL

https://reviews.freebsd.org/D29318

EMAIL PREFERENCES

https://reviews.freebsd.org/settings/panel/emailpreferences/

To: vmaffione, markj, giuseppe.lettieri_unipi.it

[vmaffione]

Forgot to link the commit to this review.

[giuseppe.lettieri_unipi.it]

It looks incorrect also to me. I think the kernel mapping is accessed by
netmap (kernel) at least to touch the netmap_ring structs, and initialize
netmap_if structs (in some cases, VALE can also touch the netmap buffers).
But I may be misunderstanding, since "extmem" is a kind of recent addition.
Maybe Giuseppe can shed a light on this matter.

You are both right and the new comment (mentioning munmap()) is correct.