On Fri, 2024-09-13 at 11:43 -0700, John Stultz wrote: > On Fri, Sep 13, 2024 at 5:01 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Fri, 2024-09-13 at 13:26 +0200, Jan Kara wrote: > > > So what would be the difference if we did instead: > > > > > > old = atomic64_read(&mg_floor); > > > > > > and not bother with the cookie? AFAIU this could result in somewhat more > > > updates to mg_floor (the contention on the mg_floor cacheline would be the > > > same but there would be more invalidates of the cacheline). OTOH these > > > updates can happen only if max(current_coarse_time, mg_floor) == > > > inode->i_ctime which is presumably rare? What is your concern that I'm > > > missing? > > > > > > > My main concern is the "somewhat more updates to mg_floor". mg_floor is > > a global variable, so one of my main goals is to minimize the updates > > to it. There is no correctness issue in doing what you're saying above > > (AFAICT anyway), but the window of time between when we fetch the > > current floor and try to do the swap will be smaller, and we'll end up > > doing more swaps as a result. > > Would it be worth quantifying that cost? > There's a patch in the larger set that adds some percpu counters to count events. One of them was successful floor value swaps. I dropped that particular counter from the v7 set, but we could resurrect it. > > Do you have any objection to adding the cookie to this API? > > My main concern is it is just a bit subtle. I found it hard to grok > (though I can be pretty dim sometimes, so maybe that doesn't count for > much :) > It seems if it were misused, the fine-grained accessor could > constantly return coarse grained results when called repeatedly with a > very stale cookie. > > Further, the point about avoiding "too many" mg_floor writes is a > little fuzzy. It feels almost like folks would need to use the cookie > update as a tuning knob to balance the granularity of their timestamps > against the cost of the global mg_floor writes. So this probably needs > some clear comments to make it more obvious. > Fair points. I don't have any hard numbers around it. I'm mainly just trying to do what I can to keep the floor swaps to an absolute minimum. This is a global value after all so we really are better off avoiding cache invalidations. That said, passing the cookie like this would only open the window a small amount. I can certainly drop that part of the interface. In the big scheme of things I doubt it'll make much difference in performance, and if it does we can always bring it back. If that sounds OK, I'll send a v8 (after some testing). I have some comment updates I'd like to add as well. -- Jeff Layton <jlayton@xxxxxxxxxx>
