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 83ED3C433FE for ; Mon, 10 Jan 2022 13:45:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCF9C6B0075; Mon, 10 Jan 2022 08:45:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D7EAC6B0078; Mon, 10 Jan 2022 08:45:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C1E7C6B007B; Mon, 10 Jan 2022 08:45:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0203.hostedemail.com [216.40.44.203]) by kanga.kvack.org (Postfix) with ESMTP id B34E86B0075 for ; Mon, 10 Jan 2022 08:45:52 -0500 (EST) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 6FB729636E for ; Mon, 10 Jan 2022 13:45:52 +0000 (UTC) X-FDA: 79014500544.23.C71CFD5 Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by imf17.hostedemail.com (Postfix) with ESMTP id A1FE34000F for ; Mon, 10 Jan 2022 13:45:51 +0000 (UTC) Received: by mail-ed1-f49.google.com with SMTP id z22so8696966edd.12 for ; Mon, 10 Jan 2022 05:45:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=bHeTRojrHT97CzcJwwp8ETZD3FH9KBheVLaCf3e+Bz0=; b=mam0LsLi8LoWTGNIdMOetg75e2XJQGJ0mNUoJa9FG0TO6dRWdqOWRNLdf+K1w2HCvn ekYd1httCsa1T8n7aCJEd2iRaN9JPVswWNi4V4+8QXBahGH9CmKwtF2fOYG5qCCzLeE+ MG5PXOzkIGYFwWUVzNWAB2+Dl6nQHonD2kk2/xKDWsDE20PchZ8sFeNsreLfHgz8BtcZ oVS/WCHrk7F2C/U0h5gntBVEI6yQZUI/Uwj0PVyMu/QaOlmYwR1GJZff4he0ywtp6D32 cJ1i3G7EOdxnqUy88mBICgzyC4FU6+rmZ1T3ihyWYr0gp2JMb52qWRbKIZujTXgqTuPK +ufA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=bHeTRojrHT97CzcJwwp8ETZD3FH9KBheVLaCf3e+Bz0=; b=VbC7ocf26Cl0PUVBBcgF+wFxHGJ6RIM69KJHTGE7/gZlE6Toju2EzTaSBciG+pl13p YoQ3AHbAji8BW7+9pkkbOQwKTQQ8+Br4jx8QLnsm3PDImjOCK8SqbVTAc0sOWds2O1XD XSZBRkywNA2cBgVUNP46cV11KUb4eOk42W0lTvoAFbZOo/97h2JNJETwr/MoTpwVUyKF qTTAFVcd5MoWjdF9bMGwTuSwxUYvJMRZuwheLkT914W7zbx1U2mHwJ/ainpo3CEtrP1n Mnvy9ibgGjw6XFWOwKGLJGQkEs4ocuyalsO73n9o944+zPMstpIz9uMESLlABKCg9sei /70g== X-Gm-Message-State: AOAM531XaWGLQHIAFzEKT5LviwtnyJU0cVOX1tCKqCUmsuAvBEb88bwf tbluySVBDA6pTJZRgr6bI80+Fg== X-Google-Smtp-Source: ABdhPJxeRE0a4BzWgsaDYzwOeQO6iPCeZjoODWDm48/LJAdqsxT5P++dvckwm+iL3KZk5c4Bb65k1Q== X-Received: by 2002:a17:907:960d:: with SMTP id gb13mr3041620ejc.572.1641822350222; Mon, 10 Jan 2022 05:45:50 -0800 (PST) Received: from localhost ([57.190.1.3]) by smtp.gmail.com with ESMTPSA id l18sm2464463ejf.7.2022.01.10.05.45.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Jan 2022 05:45:49 -0800 (PST) Date: Mon, 10 Jan 2022 14:45:41 +0100 From: Johannes Weiner To: Linus Torvalds Cc: 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 , Suren Baghdasaryan Subject: Re: psi_trigger_poll() is completely broken Message-ID: References: <000000000000e8f8f505d0e479a5@google.com> <20211211015620.1793-1-hdanton@sina.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: A1FE34000F X-Stat-Signature: e3pcxhbtgwoz37e4g4fse5skdj4orydp Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=mam0LsLi; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf17.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.208.49 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org X-HE-Tag: 1641822351-942571 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 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. 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? (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 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