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 B114FC433F5 for ; Mon, 10 Jan 2022 17:25:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 29BDB6B0071; Mon, 10 Jan 2022 12:25:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2245A6B0072; Mon, 10 Jan 2022 12:25:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 09F0E6B0074; Mon, 10 Jan 2022 12:25:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0095.hostedemail.com [216.40.44.95]) by kanga.kvack.org (Postfix) with ESMTP id E793A6B0071 for ; Mon, 10 Jan 2022 12:25:13 -0500 (EST) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id A869B18033D14 for ; Mon, 10 Jan 2022 17:25:13 +0000 (UTC) X-FDA: 79015053306.26.8EB840A Received: from mail-yb1-f181.google.com (mail-yb1-f181.google.com [209.85.219.181]) by imf15.hostedemail.com (Postfix) with ESMTP id 528EBA0014 for ; Mon, 10 Jan 2022 17:25:12 +0000 (UTC) Received: by mail-yb1-f181.google.com with SMTP id 127so23692230ybb.4 for ; Mon, 10 Jan 2022 09:25:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S49UlxL/fsiTamxTIgefeURrqnW3PFqiJDDCzTc/0cc=; b=VCdeKW3sNP+kY6RTFC/xDpcb6BGp9EFzyrlM8ZMqgpZHIL4xCzt4ONzlz0iXBKAs6A QIzGQBKrqqzWhUm+c0djm+bbedLM7nFmAKNNYybG752cXjGeMmIQVF+QhPbltPM/2VhG f7r/J8vEF3BxJ69LvJ+7jpkKy1u2C5VYnMZwlqnjVB2y1UCcgMDABX0/5ZKBQ1rj2pMF V8NwgXcMILRZCjB2iqkYE31Kfyx6mgaFxTg7QrTjO3QzrKHOxEH4eAUCbMQAFxJQEGKx 9pzy7hWJSTWJNfq2JHUcfnNXUBLNxrE1X4INdXGewmaVJH9E3UKqi5VzUvxFB7oATIfI C7UQ== 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=S49UlxL/fsiTamxTIgefeURrqnW3PFqiJDDCzTc/0cc=; b=fA/I+PCH68TPe6B/YkUDX70UJxzvHZIIPAArF9vJyTNVO16pX3lkUqY33KAkusiiGU ypX266hb5hzg1NanWTnDFBQUNpdx3OIfUZwDufLRNHK3y1iQeraN6CJRcrlhpOJ8J+nR HSyk80DxbHee3gJKazdxmIrlWOfZVfr/gCHTOWyhPsxKnHkQplsthBEsu/G5AC93sXD2 IolhKKW3ypCnnNbY5dqSRCwHAjmqwTi3bnVDhzVi/qjYsDF3a/jtI1jJpVbSS4YH4h61 2Hlh2We9ccc3TpkEtK2Nmom4j0YJ4XTpI83gyP73lWcZNqqX1n1yLpGc9PmEnzIxShYp bO2Q== X-Gm-Message-State: AOAM533D3bu+wcRb8lzj9IwTQS3zg+RMSnqpSgKXaPKL3kIcr++g8PuO XbCAQ5euGP2O31k8p5ledLXSQ3HtvaQ7li36Xfsb7w== X-Google-Smtp-Source: ABdhPJxGq+zYLLxC8sn7LZDUsLrH4GX6FkkADsYi2zUYF9m5j4rlWu6WSTZbP9nlpFwN9FhHCK4Gnu8L8/uSWMlf4HE= X-Received: by 2002:a05:6902:703:: with SMTP id k3mr758294ybt.225.1641835511962; Mon, 10 Jan 2022 09:25:11 -0800 (PST) MIME-Version: 1.0 References: <000000000000e8f8f505d0e479a5@google.com> <20211211015620.1793-1-hdanton@sina.com> In-Reply-To: From: Suren Baghdasaryan Date: Mon, 10 Jan 2022 09:25:00 -0800 Message-ID: Subject: Re: psi_trigger_poll() is completely broken To: Johannes Weiner Cc: Linus Torvalds , Eric Biggers , Tejun Heo , Zefan Li , Peter Zijlstra , Juri Lelli , Vincent Guittot , Ingo Molnar , Hillf Danton , syzbot , linux-fsdevel , Linux Kernel Mailing List , syzkaller-bugs , Linux-MM Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=VCdeKW3s; spf=pass (imf15.hostedemail.com: domain of surenb@google.com designates 209.85.219.181 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 528EBA0014 X-Stat-Signature: yjzntfpgwtt1dah9hknxujcorsdytpra X-HE-Tag: 1641835512-444451 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 Mon, Jan 10, 2022 at 5:45 AM Johannes Weiner wrote: > > On Wed, Jan 05, 2022 at 11:13:30AM -0800, Linus Torvalds wrote: > > On Wed, Jan 5, 2022 at 11:07 AM Linus Torvalds > > wrote: > > > > > > Whoever came up with that stupid "replace existing trigger with a > > > write()" model should feel bad. It's garbage, and it's actively buggy > > > in multiple ways. > > > > What are the users? Can we make the rule for -EBUSY simply be that you > > can _install_ a trigger, but you can't replace an existing one (except > > with NULL, when you close it). > > Apologies for the delay, I'm traveling right now. > > The primary user of the poll interface is still Android userspace OOM > killing. I'm CCing Suren who is the most familiar with this usecase. > > Suren, the way the refcounting is written right now assumes that > poll_wait() is the actual blocking wait. That's not true, it just > queues the waiter and saves &t->event_wait, and the *caller* of > psi_trigger_poll() continues to interact with it afterwards. Thanks for adding me, Johannes. I see where I made a mistake. Terribly sorry for the trouble this caused. I do feel bad. > > If at all possible, I would also prefer the simplicity of one trigger > setup per fd; if you need a new trigger, close the fd and open again. > > Can you please take a look if that is workable from the Android side? Yes, one trigger per fd would work fine for Android. That's how we intended to use it. I'm still catching up on this email thread. Once I digest it, will try to fix this with one-trigger-per-fd approach. About the issue of serializing concurrent writes for cgroup_pressure_write() similar to how psi_write() does. Doesn't of->mutex inside kernfs_fop_write_iter() serialize the writes to the same file: https://elixir.bootlin.com/linux/latest/source/fs/kernfs/file.c#L287 ? > > (I'm going to follow up on the static branch issue Linus pointed out, > later this week when I'm back home. I also think we should add Suren > as additional psi maintainer since the polling code is a good chunk of > the codebase and he shouldn't miss threads like these.) That would help me not to miss these emails and respond promptly. Thanks, Suren. > > > That would fix the poll() lifetime issue, and would make the > > psi_trigger_replace() races fairly easy to fix - just use > > > > if (cmpxchg(trigger_ptr, NULL, new) != NULL) { > > ... free 'new', return -EBUSY .. > > > > to install the new one, instead of > > > > rcu_assign_pointer(*trigger_ptr, new); > > > > or something like that. No locking necessary. > > > > But I assume people actually end up re-writing triggers, because > > people are perverse and have taken advantage of this completely broken > > API. > > > > Linus