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 8311DCA0FFB for ; Fri, 30 Aug 2024 14:18:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ECF576B0158; Fri, 30 Aug 2024 10:18:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E59046B0159; Fri, 30 Aug 2024 10:18:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF90F6B015A; Fri, 30 Aug 2024 10:18:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id ACAC46B0158 for ; Fri, 30 Aug 2024 10:18:27 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 5EA90AB385 for ; Fri, 30 Aug 2024 14:18:27 +0000 (UTC) X-FDA: 82509117054.19.43CD169 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf05.hostedemail.com (Postfix) with ESMTP id 562AD10001C for ; Fri, 30 Aug 2024 14:18:25 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ci3ojLNr; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf05.hostedemail.com: domain of oleg@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=oleg@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725027484; a=rsa-sha256; cv=none; b=SSnb9xGZ1ZR0EyM2zgsf/ZyRam+Tp/IXkZ+qj23/sCgzu3pkmyXG1Yo2r9TMDvnYGJ98ZR U8jO91JzQiflk0QT8n2g/x/hhXqu2luEk6c0kotgJZGclPLgecCOQTAilwsDyPwEDnsTF2 D+OXGD8pRsDasB1QI61ImZEA80ak1uM= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ci3ojLNr; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf05.hostedemail.com: domain of oleg@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=oleg@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725027484; 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=7ycVg1x9tXPFTKLRTderi7SLBgh7zP/ApOSYr7txowM=; b=oPX9VvCIzGKl/WdjLtjO0W9e5Pc8lBt4l7wpV8UofHrvepCNgN1THwNEuz+6nsrTLQDKY4 nr/ZPwByHO2lqrFEgS026yTnfSfl7fZRHDby6Q9NEiDuS/MHQpJ2QEbCf2V0Q9rV9ty2ai 5Ea9gdD3MQ5US+GMuARo/+rNUST+YMw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1725027504; h=from:from: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; bh=7ycVg1x9tXPFTKLRTderi7SLBgh7zP/ApOSYr7txowM=; b=ci3ojLNri+3v+KegGaGKZzyiCYARSEOYze9kQuWFluvVb/NbyBn6AsETpGr4sLCZ9T1Jal D/MkV80l12EQQvHNTjpTSzMVznwi0QT4A+iBustUOzsMzzXThIjbPKL2wjQoZ34TcSTX5E A2XMmoXwM3Eb0ZJOPMAnJOWvxYUGvKM= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-515-Kab89R1lP665Yn8Uhprd4Q-1; Fri, 30 Aug 2024 10:18:21 -0400 X-MC-Unique: Kab89R1lP665Yn8Uhprd4Q-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 87DE61954B2E; Fri, 30 Aug 2024 14:18:18 +0000 (UTC) Received: from dhcp-27-174.brq.redhat.com (unknown [10.45.225.148]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with SMTP id 260813001FC3; Fri, 30 Aug 2024 14:18:12 +0000 (UTC) Received: by dhcp-27-174.brq.redhat.com (nbSMTP-1.00) for uid 1000 oleg@redhat.com; Fri, 30 Aug 2024 16:18:10 +0200 (CEST) Date: Fri, 30 Aug 2024 16:18:03 +0200 From: Oleg Nesterov To: Andrii Nakryiko Cc: Jiri Olsa , Andrii Nakryiko , linux-trace-kernel@vger.kernel.org, peterz@infradead.org, rostedt@goodmis.org, mhiramat@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, paulmck@kernel.org, willy@infradead.org, surenb@google.com, akpm@linux-foundation.org, linux-mm@kvack.org Subject: Re: [PATCH v4 4/8] uprobes: travers uprobe's consumer list locklessly under SRCU protection Message-ID: <20240830141803.GB20163@redhat.com> References: <20240829183741.3331213-1-andrii@kernel.org> <20240829183741.3331213-5-andrii@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Rspam-User: X-Rspamd-Queue-Id: 562AD10001C X-Rspamd-Server: rspam01 X-Stat-Signature: s5xccoimdx49d9ztyqt1e64174eu6bu4 X-HE-Tag: 1725027505-407487 X-HE-Meta: U2FsdGVkX18kcJYS0mM3EmMPoZbjXtE0g2yIszLQp8QmcTBnHWRy9vfJnGL1Yxk4dbCRqQPpYGw44LglxpzoXmqXux4UvMrJpDPyr9T30nAmWC9xjNYDGNl0G93ADtzP/zgr438rtIV84R6rS2Q6Ur/ScVP1RJs8jo3FtrrMAOh212cD/CUW+srSdl+1Nw8gF4JGeAl/skokuAZ+4RrxP2J2HCX5NTQn0+KrWHaeE46gYwo7EzOnROaEX5dIvkwpvnYpbysyKNisFfmiR/xFymgyNugF517CaB5aPSSkrpc0r35y5evXN+y0rpA8EfEJWqeZ6cksi2b6V7sOa8tXv948NrTxnSmMdwE7RnkXsLnVqMSct4+g8rw/7FEEUQMt/cfepZMhYAfhXrfp1p+Ln8YEcrKQPurob2EYUmHsZhUc4GP3BvQ2Rk5SKHhQkN2ujsnz8Z1a99c0GhjJQ0rMB6Fvw+vAk+Zr1Al6MpdgAI70ZnCkVjVtCDpI21jKgN4ugM3Kd1r6IzGHycLTtnXHrvYGnqh9IjzqNnQyJirAF62cV0BEB2Wf0+knu14wlFJYKV4jSxDtTeSBsi4iFAnM4RFkEyeL8uX/OSse20azekjPcNmFV6yEMtd0qsbHWfi38++xNgRVMxn7jrOk8rUAPZFQpBE7rctW+Fzr4JFSIB6ObTz2QPx4jB8VjDGB0J1Az0RC8ZMFstkXboiK7yERd506znliQuDU5N2B8dFHAvb/2TS3jvHW/+ZtRXXLzsSZYxO9PeEcH6/dYXty9XYtFxjpw+gqZWsLyamsR+x6T50jwjOIAQ9dNp7RVLz1rwKX4Tzs6Pp4QLM+WwO/2Ub9iqu6hovapXYRtcKv5FTtpdTnqkd8VhhIlxUwnDUTZyJYxy1zp169QX2Zbmy85GQTrbuXy/oh/oH2L3n+f5NTqf2tTrNaqacNUjJwSzZBf+MUkFYMASMOs0m0XldYVMS hgStC0Iw 5BnhjSMTf7Ql2getqB1fyvAOYLS7xbqyMF7ziR1rRCPNiZsTgA0YvM/2yjy7akSRLW9ZB1Hy06BAjsf7tkgTxLqxDr8GIm3zKLUxbYrzTu2d05au3YHnmBwG07s5mb+m455I9Jx/Zrf5IQuG21u2bvVi5eQCPLq2rOv3gAp5egl19XPOGe9xo3yC4BazKHJrvvx47oHyIEwkwxTpyMUJd1qDoAJsI/pT14By6CYosNLFtdqpURlEcjlPaPsVl9Gp25Npb/mfC5WDVJ85yrWM6GQBlJ7w7aznmERBZeOppA+xnMT2RVq8ATnO02BKLLx4jVg6TNHdGn7aP+tEC2FtGmYbOpcfiZw8c6T4wm15OarETATwfiQ7gyFX/C9qVKys9nHDMSwdvcunyF3Aaphq1fGjTyEBSQL0XCoUZ3CJo30os/1p+sd56T2+DXA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000014, 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 08/29, Andrii Nakryiko wrote: > > On Thu, Aug 29, 2024 at 4:10 PM Jiri Olsa wrote: > > > > > @@ -2101,17 +2110,24 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs) > > > need_prep = true; > > > > > > remove &= rc; > > > + has_consumers = true; > > > } > > > current->utask->auprobe = NULL; > > > > > > if (need_prep && !remove) > > > prepare_uretprobe(uprobe, regs); /* put bp at return */ > > > > > > - if (remove && uprobe->consumers) { > > > - WARN_ON(!uprobe_is_active(uprobe)); > > > - unapply_uprobe(uprobe, current->mm); > > > + if (remove && has_consumers) { > > > + down_read(&uprobe->register_rwsem); > > > + > > > + /* re-check that removal is still required, this time under lock */ > > > + if (!filter_chain(uprobe, current->mm)) { > > > > sorry for late question, but I do not follow this change.. > > > > at this point we got 1 as handler's return value from all the uprobe's consumers, > > why do we need to call filter_chain in here.. IIUC this will likely skip over > > the removal? > > > > Because we don't hold register_rwsem we are now racing with > registration. So while we can get all consumers at the time we were > iterating over the consumer list to request deletion, a parallel CPU > can add another consumer that needs this uprobe+PID combination. So if > we don't double-check, we are risking having a consumer that will not > be triggered for the desired process. Oh, yes, but this logic is wrong in that it assumes that uc->filter != NULL. At least it adds the noticeable change in behaviour. Suppose we have a singler consumer UC with ->filter == NULL. Now suppose that UC->handler() returns UPROBE_HANDLER_REMOVE. Before this patch handler_chain() calls unapply_uprobe(), and I think we should keep this behaviour. After this patch unapply_uprobe() won't be called: consumer_filter(UC) returns true, UC->filter == NULL means "probe everything". But I think that UPROBE_HANDLER_REMOVE must be respected in this case anyway. Thanks Jiri, I missed that too :/ Oleg.