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? > 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. thanks -john
