From patchwork Thu Oct 3 20:19:31 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 280414 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id CF0562C00C7 for ; Fri, 4 Oct 2013 06:20:15 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754854Ab3JCUUN (ORCPT ); Thu, 3 Oct 2013 16:20:13 -0400 Received: from mail-ie0-f170.google.com ([209.85.223.170]:35022 "EHLO mail-ie0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755263Ab3JCUTz (ORCPT ); Thu, 3 Oct 2013 16:19:55 -0400 Received: by mail-ie0-f170.google.com with SMTP id x13so6894639ief.1 for ; Thu, 03 Oct 2013 13:19:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=nzp5Qp66JCKCkmfxGhtMrP8IKWIPtkBXHlOn5snX34A=; b=HBOSRY/pYQPlysDH4WFH1d9obcskrdA2/VAMT5/rxUKg6MqwPmTFS54mrSBrgmNAG1 XNqZ43a/tbn+hZYwtB4UnOLJjDpW7TDfWmaU018pk/rHkgHfIuzrC4W8KYlq9sXqFbIh SX0soFuW2uIjZKz2jYATcWrWQ99CS0w3iZCSRrNJ/3OOtu63AeugQt8CSPOJ8MmM9ELB 1IB71qcukrWEbfbRBFMh4qYo36DI4K0k2NqqZq3czgFlRgyrhQr8imuGTZblHJ0xo6A/ aZ6CIWMs2CxvdF22yq0AB8h5C44ubw1kIKHMKDXl9utvQljEP7asklQa0vmQs6M/staA KeQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=nzp5Qp66JCKCkmfxGhtMrP8IKWIPtkBXHlOn5snX34A=; b=iOBYHGmubrGfXcwaSALxScedpidhlRyU8StKcVY04uwDBm+RDPN1gWDN+IFNWXRxKj gxVvGV0j3mftDP9EweJlgdxDtItuDbKvM0v04HjZTAxoTGmuQTYTUWGtgvbrF4ZDkU8s 48qJMpWhaxVLPIDxAXQSpJRMhd5AqSaAwuyIVKeRUjXlK0abctnkBs6+1WGQnVIrUiyY D6zHKv+ujFz7MGyyHnHMd708Dk9/VzD6idGGpTsx7owG31AygsvqWxj0tWSXL4r5SieK 9wKK4/K+Ul9NzBoE9pRpkN86Zqd5gJ+R6Qa/ZFHsvgoz0M22fwBG0/UXSgx780u2V1lZ TNJw== X-Gm-Message-State: ALoCoQleX1JjBTwEJLBjU5rymbAZvtTppTWlFR2FNY0a72KElW1w1LB8Xg1wfp4XPieMGwwkV55kQBhEdth7LSPTXkFWcrk7ntqsRiaWO03bNxmZjsGWzXxRFpIZvakpESuP4Y3vjvcmuRHIWTcY1jVJDIWo2dt5gVRuUH+ub0GdZXZMg6I0FvHgshMFNLxMhTKEZXdwNzksCt10ZNe5jjUowdRLfPWHTw== X-Received: by 10.43.55.205 with SMTP id vz13mr1868101icb.65.1380831591848; Thu, 03 Oct 2013 13:19:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.243.104 with HTTP; Thu, 3 Oct 2013 13:19:31 -0700 (PDT) In-Reply-To: <20131002204141.GW12758@n2100.arm.linux.org.uk> References: <1380189591-1368-1-git-send-email-vfalico@redhat.com> <20130928213727.GC32063@redhat.com> <20131001055346.GA4759@google.com> <20131002204141.GW12758@n2100.arm.linux.org.uk> From: Bjorn Helgaas Date: Thu, 3 Oct 2013 14:19:31 -0600 Message-ID: Subject: Re: [PATCH v2] msi: free msi_desc entry only after we've released the kobject To: Russell King - ARM Linux Cc: Veaceslav Falico , "linux-pci@vger.kernel.org" , Neil Horman , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, Oct 2, 2013 at 2:41 PM, Russell King - ARM Linux wrote: > On Mon, Sep 30, 2013 at 10:53:46PM -0700, Bjorn Helgaas wrote: >> I think the current kobject delayed release is too aggressive, > > I don't agree with that statement, but the rest of the sentence I do: I think you're right. I think we need to keep the delayed release the way it is, and pay more attention to the way we remove things from sysfs. >> in the >> sense that even after we've released all references, the object can >> still be in sysfs, which causes future creates to fail. E.g., this >> fails: >> >> kset = kset_create_and_add("kobj_test", NULL, NULL); >> kset_unregister(kset); >> kset = kset_create_and_add("kobj_test", NULL, NULL); // FAILS >> >> when I think it should succeed. We don't have a way for the caller to >> determine when it's safe to do the second kset_create_and_add(). > > The reason this happens is that for some reason I can't fathom, the > sysfs cleanup is done when the release function is called, not when > the object is unregistered. Right. I think we might want to have kset_unregister() call kobject_del() explicitly. That way the kset is removed from sysfs immediately even if the release is delayed, and it is safe to create another kset with the same name as soon as kset_register() returns. Part of my confusion has been that I didn't connect "kobject_del()" with the removal from sysfs (even though this is well-documented in Documentation/kobject.txt). I think using kobject_del()an important part of fixing not only this MSI issue, but also races in the PCI device removal path. A name like "kobject_sysfs_unlink()" might be more descriptive and help convey the idea that this is the point where we prevent any future references via sysfs. > I can see why that's done - it is so that when a kobject is unregistered, > its sysfs entry hangs around until all the children have gone (and hence > its reference count then hits zero.) > >> After we release all references, I think it's OK for the kobject >> itself to continue to exist, i.e., we can delay calling t->release(). >> But it should be impossible to find a kobject with refcount == 0 via >> sysfs, so we should be able to create a new one with the same name. >> >> In terms of code, I'm suggesting something like the following: > > I think I can give you an ack for this - it looks sensible enough, and > should still have the intended debugging behaviour. I haven't tested > it though. > > Acked-by: Russell King Thanks. I think I'm going to propose something like the patch below instead. It solves the kset_create_and_add(), kset_unregister(), kset_create_and_add() problem as well. I think we'll have to address the MSI issue by adding kobject_del() in the appropriate places. the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/lib/kobject.c b/lib/kobject.c index 9621751..9098992 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -753,6 +753,7 @@ void kset_unregister(struct kset *k) { if (!k) return; + kobject_del(&k->kobj); kobject_put(&k->kobj); } -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in