From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932450AbaLBSpv (ORCPT ); Tue, 2 Dec 2014 13:45:51 -0500 Received: from mail-lb0-f179.google.com ([209.85.217.179]:42161 "EHLO mail-lb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754560AbaLBSpt (ORCPT ); Tue, 2 Dec 2014 13:45:49 -0500 MIME-Version: 1.0 In-Reply-To: <1417539208.1841.1.camel@sipsolutions.net> References: <5307CAA2.8060406@users.sourceforge.net> <530A086E.8010901@users.sourceforge.net> <530A72AA.3000601@users.sourceforge.net> <530B5FB6.6010207@users.sourceforge.net> <530C5E18.1020800@users.sourceforge.net> <530CD2C4.4050903@users.sourceforge.net> <530CF8FF.8080600@users.sourceforge.net> <530DD06F.4090703@users.sourceforge.net> <5317A59D.4@users.sourceforge.net> <547BC5AD.6090500@users.sourceforge.net> <1417465745.28610.0.camel@sipsolutions.net> <1417539208.1841.1.camel@sipsolutions.net> From: "Luis R. Rodriguez" Date: Tue, 2 Dec 2014 13:45:26 -0500 X-Google-Sender-Auth: Puv3f03xZkAYNY5T4sd1g2Xjw58 Message-ID: Subject: Re: net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put" To: Johannes Berg Cc: Julia Lawall , SF Markus Elfring , Lino Sanfilippo , Olof Johansson , "netdev@vger.kernel.org" , "backports@vger.kernel.org" , LKML , "kernel-janitors@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 2, 2014 at 11:53 AM, Johannes Berg wrote: > On Mon, 2014-12-01 at 21:34 +0100, Julia Lawall wrote: > >> > So this kind of evolution is no problem for the (automated) backports >> > using the backports project - although it can be difficult to detect >> > such a thing is needed. >> >> That is exactly the problem... > > I'm not convinced though that it should stop such progress in mainline. I believe this case requires a bit more information explained as to why it was explained. The "form" of change this patch has is of the type that can crash systems if the NULL pointer check on the caller implementation was only added later. We might be able to grammatically check for this situation in the future if we had a white list / black list / kernel revision where the NULL check was added but for now we don't have that and as such care is just required on the developer in consideration for backports. It should be up to the maintainer to appreciate the gains of doing something differently to make it easier for backporting. I obviously think its a good thing to consider, its extra work though, so only if the maintainer has some appreciation for backporting would this make sense. In this particular case I've reviewed Julia's concern and I've determined that the patch is safe up to at least v2.6.12-rc2 (which is where our git history begins on Linus' tree), this is because the check for NULL has been there since then: git show 1da177e drivers/pci/pci-driver.c +void pci_dev_put(struct pci_dev *dev) +{ + if (dev) + put_device(&dev->dev); +} So this type of wide collateral evolution should not cause panics. Because of this: Acked-by: Luis R. Rodriguez But note -- I still think its only good for us to vet these, if we can't why not? If the maintainer doesn't give a shit that's different, but if there are folks out there willing to help with vetting then well, why not :) PS. Including something like historical vetting as I did above on the commit log should help folks. Luis