On Tue, Jun 25, 2024 at 7:45 AM Oleg Nesterov <[email protected]> wrote: > > Again, I'll try to read (at least this) patch later this week, > just one cosmetic nit for now...
Thanks, and yep, please take your time. I understand that this is not a trivial change to a code base that has been sitting untouched for many years now. But I'd really appreciate if you can give it a through review anyways! > > On 06/24, Andrii Nakryiko wrote: > > > > + * UPROBE_REFCNT_GET constant is chosen such that it will *increment both* > > + * epoch and refcnt parts atomically with one atomic_add(). > > + * UPROBE_REFCNT_PUT is chosen such that it will *decrement* refcnt part > > and > > + * *increment* epoch part. > > + */ > > +#define UPROBE_REFCNT_GET ((1LL << 32) | 1LL) > > +#define UPROBE_REFCNT_PUT (0xffffffffLL) > > How about > > #define UPROBE_REFCNT_GET ((1ULL << 32) + 1ULL) > #define UPROBE_REFCNT_PUT ((1ULL << 32) - 1ULL) It's cute, I'll change to that. But I'll probably also add a comment with the final value in hex for someone like me (because I can reason about 0xffffffff and its effect on refcount, not so much with `(1LL << 32) - 1`. > > ? this makes them symmetrical and makes it clear why > atomic64_add(UPROBE_REFCNT_PUT) works as described in the comment. > > Feel free to ignore if you don't like it. > > Oleg. >
