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 253EBC02198 for ; Tue, 4 Feb 2025 12:13:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7F3C56B007B; Tue, 4 Feb 2025 07:13:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A3706B0083; Tue, 4 Feb 2025 07:13:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 645E36B0085; Tue, 4 Feb 2025 07:13:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 4750A6B007B for ; Tue, 4 Feb 2025 07:13:09 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id DFE1E80907 for ; Tue, 4 Feb 2025 12:13:08 +0000 (UTC) X-FDA: 83082151656.10.AB74799 Received: from mail-ed1-f46.google.com (mail-ed1-f46.google.com [209.85.208.46]) by imf14.hostedemail.com (Postfix) with ESMTP id 01AEA100010 for ; Tue, 4 Feb 2025 12:13:06 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=muVeYQnP; spf=pass (imf14.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738671187; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=bRRnjWlfIVm6w/pghY+MVcet6bWLZj7mP0DYoi0BL1c=; b=ajhmWzmPMfJ1KCTY68wHhFfmyukRgWqMOV5sSrSUsr/tvw/iIZaprhzoJQ84p8dmmnO13w 10ZqViBb7kHrUWRH5rUOA/kJF4sAygvoW4QoFFiKWwvfQqAz4vS50oqD4PaLOh3c3BlZbp dmoYk2AgMUDHx6fzQ6v4KPqY6aRpwKA= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=muVeYQnP; spf=pass (imf14.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.208.46 as permitted sender) smtp.mailfrom=mjguzik@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738671187; a=rsa-sha256; cv=none; b=cR38UrjhyMKIUBmvI297KgfYZE2wUC+K7VKg/sqy6U9utmilh7yCB+2PGnkilb68DHOOS9 tWWov98fLn2G1xPqYoymZgNb2+GDPlqlG1RXQ96tyh3ycS6YVgPsZdA3QezmyWnP7ejO8e n2O7LorfUJDXNZhN5zeQuLJpcCXhxcc= Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-5d3ecae02beso7569372a12.0 for ; Tue, 04 Feb 2025 04:13:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738671185; x=1739275985; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=bRRnjWlfIVm6w/pghY+MVcet6bWLZj7mP0DYoi0BL1c=; b=muVeYQnPToRyimIFnS66lv93iqcAPbheiNvoawvXb+ryeTJoYmnayE3+Wcj8bsBdyV FlkUhWjXqBoHUijniwQ4GYU90E7o/Mt7VpsTFOIOMfTmqDM1Rq+9ewaaWdtEpfFrwgFQ Flogn1tk+6dF3MbvdtqRNZOyA3nUngRZonLSihQqd+KmhUgDWiBpWQhaFmVqq4qMSinJ o90JTmOS+FGCKo2p+ZLkU2rNEyw0UiFz1rOAzCzPKeE4FFzSzrx8GowKrkn/ETmKL35b SFjWc9m8e/KRIAbKUKi+PgqzH+slEYz8M/YWKrfjPr6z2WNBhSYcSIWPzt9Qgr2HMLTp +hNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738671185; x=1739275985; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bRRnjWlfIVm6w/pghY+MVcet6bWLZj7mP0DYoi0BL1c=; b=Xq8FwqYCcOzJyGjLKqrTvvKNmDkNmaicvd/m/w3XNvyh0X6Ex2cPIRo1tJOrqPIhS3 b2awcqOepxoXl1MpcV/gggKP3V+2VyEaWzIzzRN4sDfO+XukY+KHxXFBDgLNp36lAqnR MisLKdWFyYaRrXfjL3kThCIOP558VROH+wOnMzmnML/2E8PjCYDl4AzmfIDKZC/Nncu9 KCA2W6ujKZNr/KRGtA56Elc/QEq18qIcfR97X+p+GE0nHk9tDRZIupzXoqm3YaK1MGec Wya5/eP8hjFcLuTfceI4iJFGLeyuDBsQklh/RTQo4yb2fhoI/4Be7cAQaxk06lZKCGj2 jrWA== X-Forwarded-Encrypted: i=1; AJvYcCXhsRZRcBuALWnuMYUjbS0GXO2T6mc06qJSZODaD9XSI83dpby2oK8y9hjoMFRt7SDqnTdMHVZmcw==@kvack.org X-Gm-Message-State: AOJu0Yz7ajbtUQqbC2Q/Ji0QdJsV2eqXXdahN2n2G3/+pAU11POmjalm LFF+rI5eUWkwy6joPC7JRG6w/Zgjhk/w5lCxy8auBD78InCduI6+A/oJLSNW05fTLlgHkWvddhU AREdGOcMoCs2M/l1n4XD4ktpwqy909FjW X-Gm-Gg: ASbGncu6Fdi4DdQkKukVTIaou2iG3O2H1MZCTFn4JHzpiNKy5lNDl1b1Q/AH++lFDrI jYb6ynQEu3AXQPHYLhBiaa6eNiY1EaPKnu6GL9c4dUFPkV0Sey2keFWjYYMnmk/RNh8I8dlY= X-Google-Smtp-Source: AGHT+IE8Pb8E8/DVhgn/1Bnw4b6MVo2ceJNIa27kOu65bnvqFs44utskf4y0UnDllMDniLmc/WtPM98e2H1LcdkVUhE= X-Received: by 2002:a05:6402:26c9:b0:5d4:1ac2:277f with SMTP id 4fb4d7f45d1cf-5dc5efc1b98mr28418744a12.9.1738671185148; Tue, 04 Feb 2025 04:13:05 -0800 (PST) MIME-Version: 1.0 References: <20250201163106.28912-1-mjguzik@gmail.com> <20250201163106.28912-4-mjguzik@gmail.com> <20250203180623.GC1003@redhat.com> <20250204112236.GA6031@redhat.com> In-Reply-To: <20250204112236.GA6031@redhat.com> From: Mateusz Guzik Date: Tue, 4 Feb 2025 13:12:53 +0100 X-Gm-Features: AWEUYZnWBw_jz5cDO1GIBMTrxH4MbqQJVNL_mw_DqrYzHljSZD2TA5sT15JmnPA Message-ID: Subject: Re: [PATCH v3 3/6] exit: postpone tty_kref_put() until after tasklist_lock is dropped To: Oleg Nesterov Cc: ebiederm@xmission.com, brauner@kernel.org, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 01AEA100010 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: z7go47kmx1y3coqycc6omguqc8cmrrqo X-HE-Tag: 1738671186-731271 X-HE-Meta: U2FsdGVkX18RnzD5cq9py9thOX++UOYl6+7giZPgfrOxlJptR5DvTplYGKLijzfN9VEQ84hB4wQRAzaEGUn8pgx9hKb7OJGGDerVhb6g0YIH5FsBEP7b/sk3yqX68FcWQUjxnJIEQ/NcmOJqgJC7yzS4QLHiubOiiGDmdMVmm99obBjOFvZl+7viiQP1+ToKJmO8cCJduW7Tp98Zo9syzNWSjhL32Dm4x984nodYY9m237/UVrhmhUOQ3ldZnrlkD7TGdtn6eDTpTwQ9I+O7KYSRiF3QTIeRt0r0+MsVmyVc/veu9HNMcQdgLx43hzLnV3TDy8xwgvkQfB40rkeyZX7+PYwHLyxBDeU5FFTNt8vzfsCEHhZ5M1hRCeZoXohqjTgoIJKKKm8Y1z5WqitRqUupaCucga5N/bNZ3SqxtbEB1Bbfmt7QnvN9zh2eSUTlw8QjdKBRuSj49HlnMosW+K1VCsJqj8hsSOZzdsOGZw6lwLkuYSOn0m3zYCdl2r2yDcrjOZn65z+LCrzr1dD2ftyaeDH2Elyoq37qdH1yhH5jOsIVWSN8u6PNZorbPeKHGkv7Lmd5y3ff1z69YNeswtaEfHCVdwo+vyBQs980IdFim24i1NuX8SNR3rD5ZmCFeueDHDsLv+vZlZGM/XZWKUEy7EOM+XLHdOzgg37zFtgBArTK3AL0B5UThCEAnPdkkIrqANn8TJqpIE+ILIKmwv0FhlixaYbcJOvPZjklI+7ZhhE/3UQF1mqJsEZTqkkLnbXif6PAlv2f7VZ1PCvtfWC6MIrHFeOLRQCF6usch5ZWrMz1JeqTP3BdM4sLZOpBLprcrxd5kpUgHCOjj72h1FxaPthJrrRkIItR1t0qhgBuka0IIfI+09BsbwnambjLrVZfliRBDJpRuu/z8CpqmdL97tK5frCopbA2XmmCcTMGd4r+PQnAeAdD0Eu32mGEfWxd9paValDe7fLhFSx UNGaU0vI rap5aqXm3z1ANsfJzlDom77MqaQdBy+2nb1/r44nXvK30mdk3odZrIpih/OQrgL0RxWnRHiH7HoQlWDZ83MW59GMCJx7BZB73N7irh3fxdeYd2HK/YUTh2YQ60gERLzLXy4yVIie+k3Tbi50NCyXlqUFK33b/6hjjP9doekg+r41C4FWB9zsHqoxTG3kunFx7iBArl3xMZv8UQLaTvara7k7NgO5mMmWE6jN/xhL99jgEVatcXBdqzedi0gd+KmIJtcNTazTW0o+S21/nKZb81XD9lp4NwMcJWO3vcnbw1zLxBeq6re4YPz8tbjcKfY7RbWQ/PhYL4dkB3VKSqNI3pf5C89kBXETmhPvvEswWZcY5wvsoQWKaMN3yl99sYbjH7h88gT0VekP7lRxgSi9GpA2QbeQQ7jVZOxmJ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000101, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Feb 4, 2025 at 12:23=E2=80=AFPM Oleg Nesterov wro= te: > > On 02/03, Mateusz Guzik wrote: > > > > On Mon, Feb 3, 2025 at 7:07=E2=80=AFPM Oleg Nesterov = wrote: > > > > > > On 02/01, Mateusz Guzik wrote: > > > > > > > > Instead of smuggling the tty pointer directly, use a struct so that= more > > > > things can be added later. > > > > > > I am not sure this particular change worth the effort, but I won't ar= gue. > > > I'd like to know what Eric thinks. > > > > > > > it trivially whacks an atomic from an area protected by a global lock > > Yes, but I am not sure this can make a noticeable difference... Let > me repeat, I am not really arguing, I leave this to you and Eric. > The patch looks correct and I have already acked it. Just it looks > "less obvious" to me than other changes in this series. Reducing lock hold time is basic multicore hygiene, doubly so for global lo= cks. Atomics are notoriously slow even if cache-hot. I agree this change in isolation is not a big deal, but there are several other "not a big deal"s in the area which will add up if sorted out. Given the triviality of the change as far as complexity goes, I would argue that's a routine patch not warranting much of a discussion (modulo the new struct, for that see below). > > And even if we do this, I am not sure we need the new > "struct release_task_post", it seems we can simply move > tty_kref_put() from __exit_signal() to release_task(), see the > patch at the end. > > Nobody can touch signal->tty after the group leader passes __exit_signal(= ), > if nothing else lock_task_sighand() can't succeed. > I wanted to keep "whack the tty" logic intact in order reduce any discussio= n. ;) This brings me to the release_task_post struct. Suppose the tty thing does not get patched at all or gets patched the way you are proposing here. I still need a way to handle the pidmap stuff. Suppose to that end a pid array gets passed directly. Some time later another thing might pop up which gets determined within the lock and wants to return the state to clean up after the unlock. Then someone will need to add a mechanism of the same sort I'm adding here. iow I'm future-proofing this and tty just happens to be the first (even if not necessary) consumer. iow preferably the struct (or an equivalent) would still be there without tty stuff. Given the nature of the change there is a lot of opinionated handwaving possible and "happy to change" vs "feel free to ignore" is not a productive discussion, thus I would like to clarify: my primary interest is to whack the pidmap thing out of tasklist_lock-protected area and this bit I'm going to argue about. Everything else is minor touch ups which I can drop without much resistance (I'll note though there is more I can submit in the area later :P), but if you genuinely want something changed, it would be best explicitly say it. As is, given the prior ack, I intend to submit v4 with minor touch ups. > But again, feel free to ignore. > > Oleg. > > --- x/kernel/exit.c > +++ x/kernel/exit.c > @@ -146,7 +146,6 @@ static void __exit_signal(struct task_st > struct signal_struct *sig =3D tsk->signal; > bool group_dead =3D thread_group_leader(tsk); > struct sighand_struct *sighand; > - struct tty_struct *tty; > u64 utime, stime; > > sighand =3D rcu_dereference_check(tsk->sighand, > @@ -159,10 +158,7 @@ static void __exit_signal(struct task_st > posix_cpu_timers_exit_group(tsk); > #endif > > - if (group_dead) { > - tty =3D sig->tty; > - sig->tty =3D NULL; > - } else { > + if (!group_dead) { > /* > * If there is any task waiting for the group exit > * then notify it: > @@ -210,10 +206,8 @@ static void __exit_signal(struct task_st > > __cleanup_sighand(sighand); > clear_tsk_thread_flag(tsk, TIF_SIGPENDING); > - if (group_dead) { > + if (group_dead) > flush_sigqueue(&sig->shared_pending); > - tty_kref_put(tty); > - } > } > > static void delayed_put_task_struct(struct rcu_head *rhp) > @@ -279,6 +273,10 @@ repeat: > proc_flush_pid(thread_pid); > put_pid(thread_pid); > release_thread(p); > + > + if (p =3D=3D leader) > + tty_kref_put(p->signal->tty); > + > put_task_struct_rcu_user(p); > > p =3D leader; > --=20 Mateusz Guzik