On Fri, Jun 27, 2025 at 07:26:22PM +0300, Jani Nikula wrote: > On Fri, 27 Jun 2025, Ville Syrjälä <[email protected]> wrote: > > On Fri, Jun 27, 2025 at 04:34:23PM +0300, Jani Nikula wrote: > >> Internally the macro has: > >> > >> #define read_poll_timeout(op, val, cond, sleep_us, timeout_us, \ > >> sleep_before_read, args...) \ > >> > >> ... > >> > >> (val) = op(args); \ > >> > >> So you do need to provide an lvalue val, and you need to be able to add > >> () after op. I think GCC allows not passing varargs. IOW you'd need to > >> implement another macro (which could be used to implement the existing > >> one, but not the other way round). > > > > Just get rid of the 'args' and 'val' and it'll work just fine. > > Then it'll be almost identical to wait_for() (basically just missing the > > increasing backoff stuff). > > > > AFAICS this thing was originally added just for reading a single > > register and checking some bit/etc, so the name made some sense. > > But now we're abusing it for all kinds of random things, so even > > the name no longer makes that much sense. > > Yeah, evolution not intelligent design. > > > So I think just something like this would work fine for us: > > LGTM, with the _atomic version for completeness.
The other differences between wait_for() and read_poll_timeout() I see are: - read_poll_timeout() always evaluates 'cond' at least twice. For some things I think it would make sense to omit 'op' entirely so we don't have to introduce pointless variables in the caller (eg. poll_timeout(, pipe_scanline_is_moving(...), ...)) but the double evaluation of 'cond' there is not desirable. Should be an easy change to make read_poll_timeout() more like wait_for() and stash the return value into a variable. - ktime_get() vs. ktime_get_raw(). I suppose it doesn't really matter too much which is used? - 'op' and 'cond' are evaluated twice during the same iteration of the loop for the timeout case in read_poll_timeout(). wait_for() is a bit more optimal here by sampling the timeout first, then doing the 'op'+'cond', and finally checking whether the timeout happened. I suppose optimizing the timeout case isn't very critical. Though the code would be a bit less repetitive, with the caveat that we need an extra variable for the timeout result. - wait_for() has an explicit compiler barrier to make sure 'cond' and the timeout evaluation aren't reordered. Though I think it's in the wrong spot for the cases where 'op' is the one that samples the thing that 'cond' checks. However I *think* ktime_get() being a function call should be enough to prevent that reordering from happening? I guess I'll see what I can cook up to make this stuff more agreeable... -- Ville Syrjälä Intel
