Page MenuHomeFreeBSD

xen/intr: move handler removal to release from unbind
AbandonedPublic

Authored by ehem_freebsd_m5p.com on Nov 7 2021, 12:50 AM.

Details

Reviewers
royger
mhorne
Summary

There are two crucial benefits. First, this makes
xen_intr_release_isrc() a full release function, early cleanup calls
will fully cleanup. Second, this paves the way for xi_cookie to move to
architecture.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 42691
Build 39579: arc lint + arc unit

Event Timeline

This is the latest in the series of slowly getting the architecture-dependent and architecture-independent sides completely separated. If the reviewers (@royger) think this is acceptable, this would allow moving xi_cookie to xai_cookie. Thus the interrupt handler cookie would isolated to the architecture-dependent side.

I'm of two minds about this. The spots where the cookie is handled are going to be largely identical, so I'm concerned putting the cookie fully on the dependent side will cause code duplication. Yet this moves closer to having the two sides fully isolated.

The most significant portion is:

	if (isrc->xi_cookie != NULL)
		xen_arch_intr_remove_handler(isrc);

moving from xen_intr_unbind() to xen_intr_release_isrc(). This also has the benefit of xen_intr_release_isrc() completely cleaning up the source. As such internal functions which know a given structure is dead and needs to be discarded don't need to call xen_intr_unbind() (notably xen_intr_bind_virq() and xen_intr_alloc_and_bind_ipi(), see D32866).

At the point when I pushed this to Phabricator I was thinking xi_cookie should perhaps move to the architecture-dependent side. Problem is the way to go with this decision is quite unclear and I'm wondering whether to abandon D32876 since the situation is so unclear right now.

  • I now suspect xi_cookie should remain on the architecture-independent side.
  • I suspect the if (isrc->xi_cookie != NULL) xen_arch_intr_remove_handler(isrc, isrc->xi_cookie); should be near the top of xen_intr_release_isrc().
  • Instead of doing a KASSERT() if an isrc still has handlers when passed to the architecture release function, the architecture portion could do a while ((hndl = CK_SLIST_FIRST(&isrc->xi_arch->ie_handlers)) != NULL) intr_event_remove_handler(hndl); and nuke any remaining handler(s).

Trying to salvage some useful portions of this. I still like having xen_intr_release_isrc() do full cleanup, but clearly I goofed which side xi_cookie was best to remain on.

Finally figured out what had been confusing here. This portion is troublesome and I now think another approach to this is rather better.