From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <20180518130413.16997-1-roman.penyaev@profitbricks.com> <20180518130413.16997-2-roman.penyaev@profitbricks.com> In-Reply-To: From: Linus Torvalds Date: Sat, 19 May 2018 14:04:10 -0700 Message-ID: Subject: Re: [PATCH v2 01/26] rculist: introduce list_next_or_null_rr_rcu() To: Roman Pen Cc: linux-block , linux-rdma , Jens Axboe , Christoph Hellwig , Sagi Grimberg , Bart Van Assche , Or Gerlitz , Doug Ledford , swapnil.ingle@profitbricks.com, danil.kipnis@profitbricks.com, jinpu.wang@profitbricks.com, Paul McKenney , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" List-ID: On Sat, May 19, 2018 at 1:25 PM Roman Penyaev < roman.penyaev@profitbricks.com> wrote: > Another one list_for_each_entry_rcu()-like macro I am aware of is used in > block/blk-mq-sched.c, is called list_for_each_entry_rcu_rr(): https://elixir.bootlin.com/linux/v4.17-rc5/source/block/blk-mq-sched.c#L370 > Can we do something generic with -rr semantics to cover both cases? That loop actually looks more like what Paul was asking for, and makes it (perhaps) a bit more obvious that the whole loop has to be done under the same RCU read sequence that looked up that first 'skip' entry. (Again, stronger locking than RCU is obviously also acceptable for the "look up skip entry"). But another reason I really dislike that list_next_or_null_rr_rcu() macro in the patch under discussion is that it's *really* not the right way to skip one entry. It may work, but it's really ugly. Again, the list_for_each_entry_rcu_rr() in blk-mq-sched.c looks better in that regard, in that the skipping seems at least a _bit_ more explicit about what it's doing. And again, if you make this specific to one particular list (and it really likely is just one particular list that wants this), you can use a nice legible helper inline function instead of the macro with the member name. Don't get me wrong - I absolutely adore our generic list handling macros, but I think they work because they are simple. Once we get to "take the next entry, but skip it if it's the head entry, and then return NULL if you get back to the entry you started with" kind of semantics, an inline function that takes a particular list and has a big comment about *why* you want those semantics for that particular case sounds _much_ better to me than adding some complex "generic" macro for a very very unusual special case. Linus