free(9) accept NULL

Ditto

hv_vmbus_channel **channels;

AFAICT this is called from the newbus attach routine, in which case IIRC you can safely use M_WAITOK.

Yup, and in any case this check should be against != NULL, because it's not a boolean type.

The void * cast is not needed

This one probably should be:

lock;
if (channels[id] == NULL) {
    unlock;
    return;
}
channels[id] = NULL;
unlock;

I'd prefer to have a NULL check before free.

Not too much related to this patch. But are we sure this is MPSAFE? In which context do we destroy the channels? swi_msg? I think the context of this lookup is in the swi_event.

Yes, you are right. I will remove it.

This is a general good style. But for this case you can see line 529 has already done "if" test, so the value should not be NULL.

I have checked style(9), and free accepts "NULL". For code simplicity purpose, I removed the NULL check.

That if test is not useful, if there are two threads run this function, will this happen?

This also relates to my question about the lookup-then-use w/o any reference counting in the swi_event.

Isn't NULL check more consistent w/ other resource cleanup in the same function? Well, after all, its your call.

Repeately setting "NULL" does not cause issues here.

I agree with you that we should align the same rule in a small scope, file level or function level. I prefer to do a code clean in the future.