Page MenuHomeFreeBSD

Handle stacked CPU binding.
AcceptedPublic

Authored by jeff on Jul 12 2019, 12:53 AM.

Details

Reviewers
jhb
kib

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 25306
Build 23969: arc lint + arc unit

Event Timeline

jeff created this revision.Jul 12 2019, 12:53 AM
jeff added reviewers: jhb, kib.Jul 12 2019, 12:54 AM

This is a small diff to fix a bug I ran into with another patch. There are very few cases where it is necessary. The other option would be to have a stack of some kind in sched_bind() but that does not seem attractive.

kib accepted this revision.Jul 12 2019, 9:11 AM
This revision is now accepted and ready to land.Jul 12 2019, 9:11 AM
jhb added a comment.Jul 12 2019, 5:21 PM

Hmm, do you require this going forward? I think it's probably fine, but changing the binding is going to possibly break some assumption in the calling code and the purpose of the panic was to force the calling code to be aware of that and handle it (e.g. by unbinding before calling bus_bind_intr() or the like so that then the calling code is aware of the change and can handle the migration if needed.)

jeff added a comment.Jul 12 2019, 8:35 PM
In D20928#453818, @jhb wrote:

Hmm, do you require this going forward? I think it's probably fine, but changing the binding is going to possibly break some assumption in the calling code and the purpose of the panic was to force the calling code to be aware of that and handle it (e.g. by unbinding before calling bus_bind_intr() or the like so that then the calling code is aware of the change and can handle the migration if needed.)

I think there is indeed an interesting architectural question here. We don't want to silently change binding which would cause difficult bugs. I'm not sure what bugs could be caused by temporarily moving and moving back though.

This patch is necessary for another patch I have not yet posted. Briefly, it runs device attach on a cpu in the domain you are attaching to so that all of the softc memory is allocated on the correct domain without explicitly asking for it. We wind our way through bus code here. In this case the high level code doesn't care that it has been moved briefly. I could possibly temporarily replace the cpuset and switch instead. This would be more of a 'prefer' than a hard binding.

jhb added a comment.Jul 12 2019, 8:48 PM

I think moving and moving back probably doesn't break anything, so if this is the simplest approach I'm ok. For NUMA allocation I was kind of hoping we could have a way other than PCPU_GET(cpuid) to specify a preferred domain when we really want NUMA allocation, but maybe binding and pcpu is simpler than the other possibilities (e.g. could you set a temporary numa policy on curthread in place of sched_bind? Not sure if we have the bits for that though.)

jeff added a comment.Jul 12 2019, 8:52 PM

The kernel memory allocators do not check the thread's numa policy. To do so on every allocation would really be prohibitively expensive. Instead the zones have simpler policies that are always enforced the same way.

I will review and see if I can accomplish my other patch with cpuset in a way that is compatible with binding. It may better reflect the optional nature of the higher level request.

jhb added a comment.Jul 12 2019, 9:48 PM

Ok. I will trust your judgement on what is the best approach.