From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 36499217918 for ; Fri, 11 Oct 2024 12:07:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728648477; cv=none; b=FAcd2xWQ0X97b817ZHY9fhmGVAsMje1Qiao+Xoz5uFFHDWXkAtjiKJlBPAyZl08oyCR/kIy4s4c3S45XOOvZZ3gRrwb3HW75JAgoj23EKCanlUd5Y92vvmJ/L8UJwjCuVMo/KATqw0v0EUPvoguYqzRzi2XMik5xQdi886l+BkY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1728648477; c=relaxed/simple; bh=MwKBr4MW13REKEdZEAqTL7D+rRGFsGjM7n9xGoHUCEw=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=HneK7fIuR/4fYPfHHfkbCfRwj8EZS2yXKUg6gvGy7PL+J2XGmMfJuA+RVzMmJP7IY0ju1QtrbOanQ1SKBCeDdOj03Nm+/KnZAjzjrcej0CN5yQDwpctL0/1FcSXMrPxwqsWDBB1e+KIkPUHUqMYOfErmWWhF2mFaGDdQQS8Ee4c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=Prh5z9Ch; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Prh5z9Ch" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1728648474; 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=MwKBr4MW13REKEdZEAqTL7D+rRGFsGjM7n9xGoHUCEw=; b=Prh5z9Ch4Obg+vN9I0GUDr9uLmOGW5i1k9JsPMtpIOOEVIC8RHQAeNTH44HLR4LtC0lxKa CAWpDlWvHpa6C+/1JDeBO5ISRvIbK5a/AySAO8siZdJVhFjDZA7QtU9swzHoHdkLURlnDD dLfysmvU2ftmf6Q4Nso3HR1IWWhl4Qc= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-81-5D_41rRLNhi3SajUfibZlA-1; Fri, 11 Oct 2024 08:07:53 -0400 X-MC-Unique: 5D_41rRLNhi3SajUfibZlA-1 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-42cb0b0514bso12204315e9.1 for ; Fri, 11 Oct 2024 05:07:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728648472; x=1729253272; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=MwKBr4MW13REKEdZEAqTL7D+rRGFsGjM7n9xGoHUCEw=; b=I/P396dg2rTxfksFWOVdYXp95Gmh7jDmjhE+ES2SrHwTnbEmzOFFaE3t3zC8+d0+AC G6Jp3iu+ijFqO/jZz1Kx5SUkOpPOdCVJe8bSNTFLHUDy6xmHjeHpfIH547KLBdo6/Q2S 6tBq6gpkZhrI1PmeiXjncbAtnuGepXf+jl5gpB8HSMR1ildD/R2nyx+DZME1mVc3iZW4 mlLPNm2p+wAleLQ9d8WCJo6j9H2oa2XWDdFNqb7ziSiDhLONC7GUlB9uhS5cgwKMKVZQ Wi1cmZY3I77PeY+QzC4qarstyrQ8eUqpJv7lmwUsIq3XUb+c+S/A10uZ4hLHHJ9qKMh4 IR0Q== X-Forwarded-Encrypted: i=1; AJvYcCWX3KsllHT7dugfToKx0qoaFZx3/pDPKBShYWAGcESI+r5ZKgYpTXaQxeCcUy3TW7hfpCXtlok=@vger.kernel.org X-Gm-Message-State: AOJu0YzPBPBX/XcMAGyLxMEPp1flYXolPRlPVRKKVCXm4O/xOdrRhpNU uW4ZKlrDebu6kF/9gM38p1uIicTaWA7cEVbKdKrFXHlRbIZI0XwWkrP0+PGSkCeH5DzJbsAst0Z 8zV6CLs/NxeVAd/LcWCDOwoJrYyKFqiQgol5su+klyRK5FU7OZ6k/fA== X-Received: by 2002:a05:600c:1d26:b0:42f:75cd:2566 with SMTP id 5b1f17b1804b1-4311deaea05mr17632145e9.2.1728648472025; Fri, 11 Oct 2024 05:07:52 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGfafWJbYY7XgafIoUBODjA0mqAG0Im27BYfV9oETn5/NEqgGUi/qx3b+BLDWe06kqudkM50g== X-Received: by 2002:a05:600c:1d26:b0:42f:75cd:2566 with SMTP id 5b1f17b1804b1-4311deaea05mr17631395e9.2.1728648471511; Fri, 11 Oct 2024 05:07:51 -0700 (PDT) Received: from eisenberg.fritz.box ([2001:16b8:3d05:4700:3e59:7d70:cabd:144b]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-431182d7929sm40692135e9.4.2024.10.11.05.07.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Oct 2024 05:07:50 -0700 (PDT) Message-ID: Subject: Re: [RFC PATCH 13/13] Remove devres from pci_intx() From: Philipp Stanner To: Alex Williamson Cc: Dan Carpenter , Damien Le Moal , Niklas Cassel , Sergey Shtylyov , Basavaraj Natikar , Jiri Kosina , Benjamin Tissoires , Arnd Bergmann , Greg Kroah-Hartman , Alex Dubov , Sudarsana Kalluru , Manish Chopra , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rasesh Mody , GR-Linux-NIC-Dev@marvell.com, Igor Mitsyanko , Sergey Matyukevich , Kalle Valo , Sanjay R Mehta , Shyam Sundar S K , Jon Mason , Dave Jiang , Allen Hubbe , Bjorn Helgaas , Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko , Jaroslav Kysela , Takashi Iwai , Mario Limonciello , Chen Ni , Ricky Wu , Al Viro , Breno Leitao , Kevin Tian , Thomas Gleixner , Ilpo =?ISO-8859-1?Q?J=E4rvinen?= , Mostafa Saleh , Andy Shevchenko , Hannes Reinecke , John Garry , Soumya Negi , Jason Gunthorpe , Yi Liu , "Dr. David Alan Gilbert" , Christian Brauner , Ankit Agrawal , Reinette Chatre , Eric Auger , Ye Bin , Marek =?ISO-8859-1?Q?Marczykowski-G=F3recki?= , Pierre-Louis Bossart , Maarten Lankhorst , Kai Vehmanen , Peter Ujfalusi , Rui Salvaterra , Marc Zyngier , linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, netdev@vger.kernel.org, linux-wireless@vger.kernel.org, ntb@lists.linux.dev, linux-pci@vger.kernel.org, linux-staging@lists.linux.dev, kvm@vger.kernel.org, xen-devel@lists.xenproject.org, linux-sound@vger.kernel.org Date: Fri, 11 Oct 2024 14:07:48 +0200 In-Reply-To: <20241010114314.296db535.alex.williamson@redhat.com> References: <20241009083519.10088-1-pstanner@redhat.com> <20241009083519.10088-14-pstanner@redhat.com> <7f624c83-115b-4045-b068-0813a18c8200@stanley.mountain> <20241010114314.296db535.alex.williamson@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.52.4 (3.52.4-1.fc40) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Thu, 2024-10-10 at 11:43 -0600, Alex Williamson wrote: > On Thu, 10 Oct 2024 11:11:36 +0200 > Philipp Stanner wrote: >=20 > > On Thu, 2024-10-10 at 11:50 +0300, Dan Carpenter wrote: > > > On Wed, Oct 09, 2024 at 10:35:19AM +0200, Philipp Stanner wrote:=C2= =A0 > > > > pci_intx() is a hybrid function which can sometimes be managed > > > > through > > > > devres. This hybrid nature is undesirable. > > > >=20 > > > > Since all users of pci_intx() have by now been ported either to > > > > always-managed pcim_intx() or never-managed > > > > pci_intx_unmanaged(), > > > > the > > > > devres functionality can be removed from pci_intx(). > > > >=20 > > > > Consequently, pci_intx_unmanaged() is now redundant, because > > > > pci_intx() > > > > itself is now unmanaged. > > > >=20 > > > > Remove the devres functionality from pci_intx(). Remove > > > > pci_intx_unmanaged(). > > > > Have all users of pci_intx_unmanaged() call pci_intx(). > > > >=20 > > > > Signed-off-by: Philipp Stanner =C2=A0=20 > > >=20 > > > I don't like when we change a function like this but it still > > > compiles fine. > > > If someone is working on a driver and hasn't pushed it yet, then > > > it's > > > probably > > > supposed to be using the new pcim_intx() but they won't discover > > > that > > > until they > > > detect the leaks at runtime.=C2=A0=20 > >=20 > > There wouldn't be any *leaks*, it's just that the INTx state would > > not > > automatically be restored. BTW the official documentation in its > > current state does not hint at pci_intx() doing anything > > automatically, > > but rather actively marks it as deprecated. > >=20 > > But you are right that a hypothetical new driver and OOT drivers > > could > > experience bugs through this change. > >=20 > > >=20 > > > Why not leave the pci_intx_unmanaged() name.=C2=A0 It's ugly and that > > > will > > > discorage > > > people from introducing new uses.=C2=A0=20 > >=20 > > I'd be OK with that. Then we'd have to remove pci_intx() as it has > > new > > users anymore. > >=20 > > Either way should be fine and keep the behavior for existing > > drivers > > identical. > >=20 > > I think Bjorn should express a preference >=20 > FWIW, I think pcim_intx() and pci_intx() align better to our naming > convention for devres interfaces. Yup, also my personal preference. But we can mark those functions as deprecated via docstring-comment. That should fullfill Damien's goal. > =C2=A0 Would it be sufficient if pci_intx() > triggered a WARN_ON if called for a pci_is_managed() device? No, I don't think that's a good idea; reason being that pci_is_managed() just checks that global boolean which we inherited from the old implementation and which should not be necessary with proper devres. The boolean is used for making functions such as pci_intx() and __pci_request_region() hybrid. So with our non-hybrid version we never need it. P. > =C2=A0 Thanks, >=20 > Alex >=20