Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221129111055.953833-1-peternewman@google.com>
 <20221129111055.953833-3-peternewman@google.com> <60e7ba72-edff-9bf7-0541-403f25b5bc51@intel.com>
In-Reply-To: <60e7ba72-edff-9bf7-0541-403f25b5bc51@intel.com>
From:   Peter Newman <peternewman@google.com>
Date:   Wed, 7 Dec 2022 12:04:47 +0100
Message-ID: <CALPaoCgZ3XDLGWs2PdRdYMY7uBmV+9uRuahJyt7rwDO96dN9KA@mail.gmail.com>
Subject: Re: [PATCH v4 2/2] x86/resctrl: IPI all online CPUs for group updates
To:     Reinette Chatre <reinette.chatre@intel.com>
Cc:     fenghua.yu@intel.com, bp@alien8.de, derkling@google.com,
        eranian@google.com, hpa@zytor.com, james.morse@arm.com,
        jannh@google.com, kpsingh@google.com, linux-kernel@vger.kernel.org,
        mingo@redhat.com, tglx@linutronix.de, x86@kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Reinette,

On Tue, Dec 6, 2022 at 7:57 PM Reinette Chatre
<reinette.chatre@intel.com> wrote:
> On 11/29/2022 3:10 AM, Peter Newman wrote:
> > Removing a CTRL_MON or MON group directory moves all tasks to the parent
> > group. The rmdir implementation therefore interrupts any running
> > tasks which were in the deleted group to update their CLOSID/RMID to
> > those of the parent.
> >
> > The rmdir operation iterates over all tasks in the deleted group while
> > read-locking the tasklist_lock to ensure that no newly-created child
> > tasks remain in the deleted group.
>
> The above describes the current behavior. This is great context. What
> follows in the changelog is a description of different fixes. This is
> unexpected because there is no description of a problem with the current
> behavior.
>
> Could you please describe the problem with the current implementation? Next
> you could state the two possible solutions and then I think the reader would
> be ready to parse what is written below.

Ok

> > Calling task_call_func() to perform
> > the updates on every task in the deleted group, similar to the recent
> > fix in __rdtgroup_move_task(), would result in a much longer
> > tasklist_lock critical section.
>
>
> I so still think it would help to state that this additional locking
> does not help to provide precise CPU mask. Especially since
> the next paragraph may be interpreted that a precise CPU mask
> is lost by giving up the additional locking.

Yes, that's a very good point, and I'm afraid I've already made you
reiterate it once before. I will make sure to work it into the next
revision.

-Peter