Looks good to me.

Looks good, but while at hyper-v I'd also do it for hv_pci_delete_device().

You could also add the explicit topology locking into sysctl_hdac_pindump() and hdaa_sysctl_reconfig(), and remove CTLFLAG_NEEDGIANT from all 3 places in hda(4) driver. Or I can do it separately after, if you prefer.

Hans, I think you are too picky here. We already have xpt_topo_lock in CAM, ng_topo_lock in NetGraph, and 4 more line that in random drivers, so I think this abbreviation is quite typical.

I think we should drain after the wait, since some operations may still complete while we are waiting, or it may take time for consumers to release us, so why not operate the motor in process?

I am mostly unfamiliar with miibus interface, but I see ~80 calls to mii_attach() in a tree, many of which are going from driver attach methods and are already under the lock. Are you expecting to recurse on the lock?

Thank you. There is one more in altq_subr.c in init_machclk_setup().

I think detach method should be aggressive, not looking for busy, since in many cases its call means the device is no longer there or soon won't be. So you should just call fd_detach_geom as it was and then wait for providergone GEOM callback to be called. That callback is a relatively new thing, it was not there originally, that is why open counting was used.

In D31052#717780, @hselasky wrote:

In D31052#717779, @mav wrote:

Are you familiar with sys/dev/usb/wlan code, or we need somebody from wlan area, same as for rtwn and wtap?

Yes, but mostly the USB part. All drivers have a softc and each softc has a mutex which you can use for the callouts, but I don't know about LOR which might happen inside the WLAN core code.

In D31052#717780, @hselasky wrote:

I always thought it was overhead to use "!TAILQ_EMPTY()" and then "TAILQ_FIRST()" to get the first entry.

In D31052#717772, @hselasky wrote:

@mav : The best is to get rid of Giant.

I think this should better be postponed a bit until we fix all other Giant consumers except newbus, assuming it is what causes the longest delays.

I agree with Konstantin. Just recently without seeing this review I've fixes several most annoying to me cases of Giant callouts. After that quick search shows me only 38 remaining instances of callout_reset(..., 0), out of which only 6 seem like really use Giant, while most others I bet just need quick code look to be marked mpsafe.

Minor tuning to unlock_spin().

Provide VMD dma tag to all children devices, since host should see it as originator for all requests. It fixes operation with IOMMU enabled at least for directly connected devices. Devices behind a bridge still don't work right for me for some reason, despite receiving the same tag and IOMMU not reporting violations.

Limit the end of bus numbers range considering available BAR0 size.

In D31726#716029, @smalukav_gmail.com wrote:

In D31726#715924, @mav wrote:

While it looks OK to me, I think it would be good for different drivers to be as close as possible. And I see da got their locking slightly different in af1823cde891.

Do you mean using cam_periph_acquire and cam_periph_release_locked? I suppose we should ref the periph in cdsysctlinit. We use the same pattern in cdopen https://reviews.freebsd.org/source/src/browse/main/sys/cam/scsi/scsi_cd.c$756 .

While it looks OK to me, I think it would be good for different drivers to be as close as possible. And I see da got their locking slightly different in af1823cde891.

I hit this area of residual data handling in GEOM several times over the years. It would be good to formalize and make use of it somehow. But I never saw any reasonable use for it with any block device. It could possibly be used to specify how much data were read/written before some error occurred, but the problem is that in case of parallel execution, done by many classes bio_completed may not mean the first number of bytes of the request has completed, but SOME number of bytes. So it is pretty much useless from that perspective.

Thinking about it again, pcib_expand_window() should receive not the requested start/end values, but MIN()/MAX() respectively from them and the current window base/limit. Otherwise in case of multiple resources pcib_expand_window() panic on attempt to shrink the window on another side of where it growth. Unfortunately I don't have hardware configuration with more than one second level bridge connected to the one closest to the root to illustrate that.