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 4F883C433F5 for ; Thu, 25 Nov 2021 17:29:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9E11D6B0074; Thu, 25 Nov 2021 12:29:11 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 990516B0075; Thu, 25 Nov 2021 12:29:11 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 857C16B0078; Thu, 25 Nov 2021 12:29:11 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0148.hostedemail.com [216.40.44.148]) by kanga.kvack.org (Postfix) with ESMTP id 75E456B0074 for ; Thu, 25 Nov 2021 12:29:11 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 3E6C61803F488 for ; Thu, 25 Nov 2021 17:29:01 +0000 (UTC) X-FDA: 78848138082.23.AB6B075 Received: from mail-ua1-f50.google.com (mail-ua1-f50.google.com [209.85.222.50]) by imf11.hostedemail.com (Postfix) with ESMTP id C362AF0000B0 for ; Thu, 25 Nov 2021 17:29:00 +0000 (UTC) Received: by mail-ua1-f50.google.com with SMTP id p2so13653301uad.11 for ; Thu, 25 Nov 2021 09:29:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=posk.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IBEhpObGolU+4iX425jtNXgHUPZE0af1hS/Di57iqGg=; b=VpD4zBOm32koE2rSOjnPeQlqduvJvx2XL4+eWuL0MEtVlhKsiAsvCCKAhR0VUn9nLN AaIvXd/yfN3QcYIPZNx+RzfpZL7bZorzathuzF6Az0M/yB/zdNoCq9lyIomAqcuICGMQ DcOY3QreN28RCyyJIew5yWArmHaaZbg1UFZbbqClOatUTu39mwIqhmQc4oygx/bF2zpD HSmuIm1uA7/I3fKucnDqPLGexa9YIXdaS73bRUgG4/sz2TSagiglx875frKiicUw5Jaq 4DpctpHMhm8SOxcI6Ho5EF3UY5HkksBS6HfJxpod6XSwdjNBxAbH9DJuzGofR1KCiMYv tfng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IBEhpObGolU+4iX425jtNXgHUPZE0af1hS/Di57iqGg=; b=rjdyUREJggOiW+qO/LuMMvfcfHdHv3QgnvKrWrwXzH7pC95zyC6I3NPwOwgJcuW7Mk UbZkqKKBHfJcDSFCCFCdh8JcbImOfBaZXIRp0g4updMtjw47HgRDM4LqbsVEn78mlWDJ pEwogx4WmCPGv1hX6htz+zYg5ugUgUDVzH4tCwi6RmdwlwDKEpoalAYN51JNjvouKiUV jc9ISt2Zca2T3j09dhQENi6FaO+LQO6br4rfL8ekaJNTdFNRHUJEuT6crgq2LIk5verj QzVrBUBaofqBdVSLToz204vXz1SMYS75+bp3IP5kGb7IkGylLxtPGQvHznpHFFMiCJat CDOw== X-Gm-Message-State: AOAM531AuNjqorQyz+nDxwVLk6fuZSFDTudaCC2RaBi9EhZ6a9D8NBmw ihckRk2yhqiHDuQasw9sOy0wpkxnWfx1VL6/OTo1sQ== X-Google-Smtp-Source: ABdhPJzuLRYrRCP8uns6IAySz1dNEKf0WKtd4iu5aTRnogqI8B4oQPNKXVSuIYW162s/1xr+kyIDby8hCr6rYJsVYh4= X-Received: by 2002:a67:bb19:: with SMTP id m25mr11744019vsn.47.1637861339950; Thu, 25 Nov 2021 09:28:59 -0800 (PST) MIME-Version: 1.0 References: <20211122211327.5931-1-posk@google.com> <20211122211327.5931-4-posk@google.com> <20211124200822.GF721624@worktop.programming.kicks-ass.net> In-Reply-To: <20211124200822.GF721624@worktop.programming.kicks-ass.net> From: Peter Oskolkov Date: Thu, 25 Nov 2021 09:28:49 -0800 Message-ID: Subject: Re: [PATCH v0.9.1 3/6] sched/umcg: implement UMCG syscalls To: Peter Zijlstra Cc: Ingo Molnar , Thomas Gleixner , Andrew Morton , Dave Hansen , Andy Lutomirski , Linux Memory Management List , Linux Kernel Mailing List , linux-api@vger.kernel.org, Paul Turner , Ben Segall , Peter Oskolkov , Andrei Vagin , Jann Horn , Thierry Delisle Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C362AF0000B0 Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=posk.io header.s=google header.b=VpD4zBOm; dmarc=none; spf=pass (imf11.hostedemail.com: domain of posk@posk.io designates 209.85.222.50 as permitted sender) smtp.mailfrom=posk@posk.io X-Stat-Signature: zgha8chjpchps6dg7m6d8tuuehcn9heg X-HE-Tag: 1637861340-715390 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: Thanks, Peter, for the review! Some of your comments, like ratelimiting pr_warn and removing gotos, are obvious in how to address them, so I'll just do that and won't mention them here. Some comments are less clear re: what should be done about them, so I have them below with my own comments/questions. At a higher level, I get that the uaccess patch is bad and needs serious changes. But based on your comments on this main patch so far, it looks like the overall approach did not raise many objections - is it so? Have you finished reviewing the patch? Please also look at my questions/comments below. Thanks, Peter [...] > > +struct umcg_task { [...] > > The thing is; I really don't see how this is supposed to be used. Where > did the blocked and runnable list go ? > > I also don't see why the kernel cares about idle workers at all; that > seems something userspace can sort itself just fine. > > The whole next_tid thing seems confused too, how can it be the next task > when it must be the server? Also, what if there isn't an idle server? > > This just all isn't making any sense to me. Based on your later comments I assume it is clearer now. The doc patch 5 has a lot of extra explanations and examples. Please let me know if something is still unclear here. > I'm still very hesitant to use ktime (fear the HPET); but I suppose it > makes sense to use a time base that's accessible to userspace. Was > MONOTONIC_RAW considered? I believe it was considered. I'll re-consider it, and add a comment if the new consideration arrives at the same conclusion. > Using CLOCK_REALTIME timers while the rest of the thing runs off of > CLOCK_MONOTONIC doesn't seem to make sense to me. Why would you want to > have timeouts subject to DST shifts and crap like that? Yes, these should be the same if at all possible. I'll definitely reconsider what clock to use in both timeouts and state timestamps. > Oooh, someone made things super confusing by doing s/runnable/idle/ on > the whole thing :-( That only took me most of the day to figure out. > Naming is important, don't mess about with stuff like this. I clearly remember I had four states: blocked, pending, runnable, running (I still believe that four states better reflect what is going on here). The current blocked/idle/running is the result of an early discussion. Something along the lines of: pending workers (=unblocked workers that the userspace still thinks are blocked) are better named as idle; also the kernel does not really care about what userspace thinks, so idle workers and runnable workers are the same from the kernel point of view, so let's have one state for these workers, not two. Please let me know if you want me to change anything here. I'll gladly name workers on the idle worker list as idle (or whatever you prefer), and workers that the userspace took out of the list as "runnable". Just as a FYI, workers blocked in umcg_wait() will also be called "runnable" then, as they are sitting in umcg_idle_loop() and can be woken or swapped into.