> On Aug 2, 2019, at 3:22 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote: > >> On Fri, 2 Aug 2019, Paolo Bonzini wrote: >>> On 01/08/19 23:47, Thomas Gleixner wrote: >>> Right you are about cond_resched() being called, but for SRCU this does not >>> matter unless there is some way to do a synchronize operation on that SRCU >>> entity. It might have some other performance side effect though. >> >> I would use srcu_read_unlock/lock around the call. >> >> However, I'm wondering if the API can be improved because basically we >> have six functions for three checks of TIF flags. Does it make sense to >> have something like task_has_request_flags and task_do_requests (names >> are horrible I know) that can be used like >> >> if (task_has_request_flags()) { >> int err; >> ...srcu_read_unlock... >> // return -EINTR if signal_pending >> err = task_do_requests(); >> if (err < 0) >> goto exit_no_srcu_read_unlock; >> ...srcu_read_lock... >> } >> >> taking care of all three cases with a single hook? This is important >> especially because all these checks are done by all KVM architectures in >> slightly different ways, and a unified API would be a good reason to >> make all architectures look the same. >> >> (Of course I could also define this unified API in virt/kvm/kvm_main.c, >> so this is not blocking the series in any way!). > > You're not holding up something. Having a common function for this is > definitely the right approach. > > As this is virt specific because it only checks for non arch specific bits > (TIF_NOTIFY_RESUME should be available for all KVM archs) and the TIF bits > are a subset of the available TIF bits because all others do not make any > sense there, this really should be a common function for KVM so that all > other archs which obviously lack a TIF_NOTIFY_RESUME check, can be fixed up > and consolidated. If we add another TIF check later then we only have to do > it in one place. > > If we add a real API for this, can we make it, or a very similar API, work for exit_to_usermode_loop() too? Maybe: bool usermode_work_pending(); bool guestmode_work_pending(); void do_usermode_work(); void do_guestmode_work(); The first two are called with IRQs off. The latter two are called with IRQs on.