From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 222A0C433F5 for ; Sun, 6 Mar 2022 14:07:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3531A6B0071; Sun, 6 Mar 2022 09:07:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2DD046B0072; Sun, 6 Mar 2022 09:07:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1303D6B0073; Sun, 6 Mar 2022 09:07:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id F09346B0071 for ; Sun, 6 Mar 2022 09:07:07 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id BEC33813B8 for ; Sun, 6 Mar 2022 14:07:07 +0000 (UTC) X-FDA: 79214138094.12.9584389 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) by imf31.hostedemail.com (Postfix) with ESMTP id 48EE920002 for ; Sun, 6 Mar 2022 14:07:07 +0000 (UTC) Received: by mail-pj1-f48.google.com with SMTP id cx5so11112498pjb.1 for ; Sun, 06 Mar 2022 06:07:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=Ne/srXllW5XhA4je5FxIoFQvDEfC1aYr6Qjodnicalw=; b=Of21wL8noNb8+DiQW45qxkn3DvZuDRQ4knJNvDONsiJXsLfDWB7fuvXNrA62lKjrrQ X+a4mG7Re0rSfRy5w9HmQFSrt3Xp4xlWW9WPU0XvtMUYArwBQWBLz7rgHFXK1imf/KOv aMOiyuqIP6tXJ/vqjB1G6YTxHOOGPRRicUV3048ZJH9AA4leUiwgcVx6nOWz4F5qTefr yAIu/DKvVwwyApUEX9KN2jKdlifuYfbo1XQNkWCp8JIyWJbZaLwJY1gxw5mVWbFKloMh 2zvRJnb0xIUIrtdCRhStv5IsOQ2YPqs43n1XVq20tPZtAUeNHHtjjE/UyLeAR1HjuJlq +I3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=Ne/srXllW5XhA4je5FxIoFQvDEfC1aYr6Qjodnicalw=; b=qynKNKgf4Wb51gh0B38jm8fBgxaHUA2h9ARVasorbrjowYgGI2177ccrwSBUts2ko2 SlosSFFuDDIZvEMhk303brHJwYwqmCxz6GHUo7r2z7CIrCMd+UMtOVXtc71H23tEsykh 5oAO6ZMxyJGLze5ULWLH7uuZzDV92+jUh03sAy2khGtG4xChRpB8tQQgZaGkdFSnjjmW fL3HpIcKZQ9ROkwS6i525P65Zsvh5TKG8UoYqmpDHRpkQvYcsZAvCkmO839za8RqJNPh 5OZT41S9fdRIWpfKFfQSaVQzBDGeIDuqj0syjz0bB0IpPU2QNTlKzrbaiLmcg6pGAsAB 5I4w== X-Gm-Message-State: AOAM533Eb5SIWT65+VIG2qEVTWRyQiXeQzutN/uqwKz4Sb+7g/iVifiD 6xSDX3xgF7SYyzrz5WoKHok= X-Google-Smtp-Source: ABdhPJxU1K2NBiGk9ibM3CEkcT5aNEp7zE82ixtszkyo4ESEXjpxTfn7bWoRgG5ED/ZqOhZHxsCpbw== X-Received: by 2002:a17:902:e943:b0:14f:4a2b:203 with SMTP id b3-20020a170902e94300b0014f4a2b0203mr7859228pll.113.1646575625956; Sun, 06 Mar 2022 06:07:05 -0800 (PST) Received: from localhost.localdomain ([115.195.172.162]) by smtp.googlemail.com with ESMTPSA id j14-20020aa78dce000000b004f6db5478c3sm4807040pfr.131.2022.03.06.06.06.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 06 Mar 2022 06:07:05 -0800 (PST) From: Xiaomeng Tong To: torvalds@linux-foundation.org Cc: arnd@arndb.de, gregkh@linuxfoundation.org, jakobkoschel@gmail.com, jannh@google.com, keescook@chromium.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, xiam0nd.tong@gmail.com Subject: Re: [PATCH 2/6] list: add new MACROs to make iterator invisiable outside the loop Date: Sun, 6 Mar 2022 22:06:55 +0800 Message-Id: <20220306140655.19177-1-xiam0nd.tong@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: References: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 48EE920002 X-Stat-Signature: 9jgyj8jk9je6du4pxst5yj7zeqmp79y5 Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Of21wL8n; spf=pass (imf31.hostedemail.com: domain of xiam0nd.tong@gmail.com designates 209.85.216.48 as permitted sender) smtp.mailfrom=xiam0nd.tong@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-HE-Tag: 1646575627-897571 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, 5 Mar 2022 16:35:36 -0800 Linus Torvalds wrote: > On Sat, Mar 5, 2022 at 1:09 PM Linus Torvalds > wrote: > > > > Now, I'd love for the list head entry itself to "declare the type", > > and solve it that way. That would in many ways be the optimal > > situation, in that when a structure has that > > > > struct list_head xyz; > > > > entry, it would be lovely to declare *there* what the list entry type > > is - and have 'list_for_each_entry()' just pick it up that way. > > > > It would be doable in theory - with some preprocessor trickery [...] > > Ok, I decided to look at how that theory looks in real life. > > The attached patch does actually work for me. I'm not saying this is > *beautiful*, but I made the changes to kernel/exit.c to show how this > can be used, and while the preprocessor tricks and the odd "unnamed > union with a special member to give the target type" is all kinds of > hacky, the actual use case code looks quite nice. > > In particular, look at the "good case" list_for_each_entry() transformation: > > static int do_wait_thread(struct wait_opts *wo, struct task_struct *tsk) > { > - struct task_struct *p; > - > - list_for_each_entry(p, &tsk->children, sibling) { > + list_traverse(p, &tsk->children, sibling) { > > IOW, it avoided the need to declare 'p' entirely, and it avoids the > need for a type, because the macro now *knows* the type of that > 'tsk->children' list and picks it out automatically. > > So 'list_traverse()' is basically a simplified version of > 'list_for_each_entry()'. > Yes, brilliant! It is tricky and hacky. In your example: &tsk->children will be expanded to &tsk->children_traversal_type. And it also reduces column of the calling line with simplified version of list_for_each_entry. But, maybe there are some more cases the union-based way need to handle. Such as, in your example, if the &HEAD passing to list_for_each_entry is *not* "&tsk->children", but just a *naked head* with no any extra information provoided: void foo(...) { bar(&tsk->children); } noinline void bar(struct list_head *naked_head) { struct task_struct *p; list_for_each_entry(p, naked_head, sibling) { ... } } you should change all declares like "struct list_head" here with the union one, but not only in the structure of task_struct itself. I'm going to dig into this union-base way and re-send a patch if necessary. > That patch also has - as another example - the "use outside the loop" > case in mm_update_next_owner(). That is more of a "rewrite the loop > cleanly using list_traverse() thing, but it's also quite simple and > natural. > Yes, the "c" is as the found entry for outside use -- it is natural. And the "pos" as a inside-defined variable -- it is our goal. And the "continue" trick to reduce 1 line -- it is nice. > One nice part of this approach is that it allows for incremental changes. > > In fact, the patch very much is meant to demonstrate exactly that: > yes, it converts the uses in kernel/exit.c, but it does *not* convert > the code in kernel/fork.c, which still does that old-style traversal: > > list_for_each_entry(child, &parent->children, sibling) { > > and the kernel/fork.c code continues to work as well as it ever did. > > So that new 'list_traverse()' function allows for people to say "ok, I > will now declare that list head with that list_traversal_head() macro, > and then I can convert 'list_for_each_entry()' users one by one to > this simpler syntax that also doesn't allow the list iterator to be > used outside the list. > Yes, i am very glad that you accepted and agreed my suggestion for the *incremental changes* part, just like my "_inside" way used. It means a lot for me to have your approval. > What do people think? Is this clever and useful, or just too subtle > and odd to exist? > > NOTE! I decided to add that "name of the target head in the target > type" to the list_traversal_head() macro, but it's not actually used > as is. It's more of a wishful "maybe we could add some sanity checking > of the target list entries later". > > Comments? > > Linus -- Xiaomeng Tong