mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yiwei Lin <s921975628@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Yiwei Lin <s921975628@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] rbtree: update augmented data on the way down in rb_add_augmented_cached()
Date: Tue, 29 Sep 2026 03:15:26 +0800	[thread overview]
Message-ID: <20260928191526.645160-1-s921975628@gmail.com> (raw)
In-Reply-To: <20260928140513.GF4121620@noisy.programming.kicks-ass.net>

On 2026/09/28 22:05, Peter Zijlstra wrote:
> > And that is *very* close to being generalizable, obviating the need for
> > RBCOMPUTE. Does your LLM see a way to make that happen?
>
> Perhaps by doing something like so?
[...]
>  #define RB_DECLARE_CALLBACKS_MULTI(RBSTATIC, RBNAME,			\
> -			     RBSTRUCT, RBFIELD, RBCOPY, RBCOMPUTE)	\
> +			     RBSTRUCT, RBFIELD, RBCOMPUTE, RBAUG...)	\

Yes, and since RBCOMPUTE is always "reset the fields to the node's own contribution,
then fold in each child". I think we can also generalize it with RBMERGE, by
adding one extra RBINIT:

        static inline void min_vruntime_init(struct sched_entity *se)
        {
                se->min_vruntime = se->vruntime;
                se->min_slice = se->slice;
                se->max_slice = se->slice;
        }

So the original RBCOMPUTE can be generated by the template instead, which allow us to
remove min_vruntime_update.

        static inline bool RBNAME ## _compute(RBSTRUCT *node)                         \
        {                                                                             \
                typeof(node->RBAUGMENTED) aug;                                        \
                RBSTRUCT *child;                                                      \
                                                                                      \
                RBINIT(node, &aug);                                                   \
                if (node->RBFIELD.rb_left) {                                          \
                        child = rb_entry(node->RBFIELD.rb_left, RBSTRUCT, RBFIELD);   \
                        RBMERGE(&aug, &child->RBAUGMENTED);                           \
                }                                                                     \
                if (node->RBFIELD.rb_right) {                                         \
                        child = rb_entry(node->RBFIELD.rb_right, RBSTRUCT, RBFIELD);  \
                        RBMERGE(&aug, &child->RBAUGMENTED);                           \
                }                                                                     \
                if (!memcmp(&aug, &node->RBAUGMENTED, sizeof(aug)))                   \
                        return true;                                                  \
                node->RBAUGMENTED = aug;                                              \
                return false;                                                         \
        }                                                                             \

Base on this I have another idea: can we let augmented data becomes one member and
init/merge just work on its type by value. Taking sched for example:

        struct_group_tagged(sched_aug, aug,
                u64 min_vruntime;
                u64 min_slice;
                u64 max_slice;
        );

So fair.c's se->min_vruntime etc. can stay as they are, we don't have to rely on the
FOR_EACH machinery. Do you think this can be better or do you prefer the field-list form?

Thanks,
Yiwei Lin

  parent reply	other threads:[~2026-09-28 19:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 12:26 [PATCH 0/3] rbtree: fix rb_add_augmented_cached() descent and exercise rb_add*() helpers in rbtree_test Yiwei Lin
2026-09-28 12:26 ` [PATCH 1/3] rbtree_test: use rb_add() and rb_add_cached() for the basic tests Yiwei Lin
2026-09-28 12:26 ` [PATCH 2/3] rbtree: update augmented data on the way down in rb_add_augmented_cached() Yiwei Lin
2026-09-28 13:37   ` Peter Zijlstra
2026-09-28 14:05     ` Peter Zijlstra
2026-09-28 15:00       ` Peter Zijlstra
2026-09-28 19:15       ` Yiwei Lin [this message]
2026-09-28 21:37       ` Peter Zijlstra
2026-09-28 12:26 ` [PATCH 3/3] rbtree_test: use rb_add_augmented_cached() for the cached augmented test Yiwei Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260928191526.645160-1-s921975628@gmail.com \
    --to=s921975628@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=dave@stgolabs.net \
    --cc=juri.lelli@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®