Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 10 Aug 2019 00:44:58 +0000
From:   Wei Yang <richard.weiyang@gmail.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Wei Yang <richard.weiyang@gmail.com>,
        "Schmid, Carsten" <Carsten_Schmid@mentor.com>,
        "bp@suse.de" <bp@suse.de>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bhelgaas@google.com" <bhelgaas@google.com>,
        "osalvador@suse.de" <osalvador@suse.de>,
        "rdunlap@infradead.org" <rdunlap@infradead.org>,
        "richardw.yang@linux.intel.com" <richardw.yang@linux.intel.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Subject: Re: Resend [PATCH] kernel/resource.c: invalidate parent when freed
 resource has childs
Message-ID: <20190810004458.mio4vp3nk6jl2hyh@master>
Reply-To: Wei Yang <richard.weiyang@gmail.com>
References: <1565278859475.1962@mentor.com>
 <1565358624103.3694@mentor.com>
 <20190809223831.fk4uyrzscr366syr@master>
 <CAHk-=wi_9sdMxurjZ1MbNnxt-pA=dqoyf8Fdn9aYc8xvjjnTBg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHk-=wi_9sdMxurjZ1MbNnxt-pA=dqoyf8Fdn9aYc8xvjjnTBg@mail.gmail.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 09, 2019 at 03:45:59PM -0700, Linus Torvalds wrote:
>On Fri, Aug 9, 2019 at 3:38 PM Wei Yang <richard.weiyang@gmail.com> wrote:
>>
>> In theory, child may have siblings. Would it be possible to have several
>> devices under xhci-hcd?
>
>I'm less interested in the xhci-hcd case - which I certainly *hope* is
>fixed already? - than in "if this happens somewhere else".
>

Agree, this is what I want to say.

>So if we do want to remove the parent (which may be a good idea with a
>warning), and want to make sure that the children are really removed
>from the resource hierarchy, we should do somethiing like
>
>  static bool detach_children(struct resource *res)
>  {
>        res = res->child;
>        if (!res)
>                return false;
>        do {
>                res->parent = NULL;
>                res = res->sibling;
>        } while (res);
>        return true;
>  }
>
>and then we could write the __release_region() warning as
>
>        /* You should not release a resource that has children */
>        WARN_ON_ONCE(detach_children(res));
>

I am thinking about why this could happen.

To guard the core kernel code, it looks reasonable.

>or something?
>
>NOTE! The above is entirely untested, and written purely in my mail
>reader. It might be seriously buggy, including not compiling, or doing
>odd things. See it more as a "maybe something like this" code snippet
>example than any kind of final form.
>
>               Linus

-- 
Wei Yang
Help you, Help me