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;
Message-ID: <8e8202e2-6e28-61f0-b6b1-d3f782ba90f7@amd.com>
Date:   Tue, 23 Aug 2022 10:27:07 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [RFC v2] perf: Rewrite core context handling
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     acme@kernel.org, alexander.shishkin@linux.intel.com,
        jolsa@redhat.com, namhyung@kernel.org, songliubraving@fb.com,
        eranian@google.com, alexey.budankov@linux.intel.com,
        ak@linux.intel.com, mark.rutland@arm.com, megha.dey@intel.com,
        frederic@kernel.org, maddy@linux.ibm.com, irogers@google.com,
        kim.phillips@amd.com, linux-kernel@vger.kernel.org,
        santosh.shukla@amd.com, ravi.bangoria@amd.com
References: <20220113134743.1292-1-ravi.bangoria@amd.com>
 <YqdLH+ZU/sf4n0pa@hirez.programming.kicks-ass.net>
 <YqdL8LsOvxNqhz/v@hirez.programming.kicks-ass.net>
 <f994d403-df7b-d88e-8324-c29d0ef2034e@amd.com>
 <YwO0Nh5LHZ7uzWvm@worktop.programming.kicks-ass.net>
From:   Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <YwO0Nh5LHZ7uzWvm@worktop.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZDhXQ1pRM3dEeTA4REk1L3Ftc2hxc0R3bDVrRGFFbVFyWGZLbEE5aURPcm5n?=
 =?utf-8?B?UlYzL2RzQmR3aU1uY1p3ZE1zcUJjb3lESFNydURCQUJxMzBCNTdjKzA4ckdr?=
 =?utf-8?B?a0NkSlV5YmdSRmRpQzdtZXBXS0toaCtMYTFDd3FBWU9LOHpoWDN3SFB3TGVV?=
 =?utf-8?B?byt5SS9xVGxvZFdPUTZMcGQ2SmdSOHlDRHQ1em9wZzgyU2ZZVWo0U3RmUTZk?=
 =?utf-8?B?emFXMGw3WGJaeHF0MzJZWmxLZENHNGRRY1lWYmdPTkgvMzdPaDRjamtIUVdW?=
 =?utf-8?B?VFpVdExnamRndDFrSCsxZ2s5eU1TSzZQNTNMRE5kT3diaFVkNUpaRXhUMkpX?=
 =?utf-8?B?aFBqU2tPTExuSFdsZUFzNnRmZFJhc3Jkd1FaREJWWC8wQ05PeUNYRjVDVkxE?=
 =?utf-8?B?dDQvUDJqTlRXV1NRTGVKY3VnbXI3azMzOVQrV2lZV3c0ZDhoVjlndDZtMU1o?=
 =?utf-8?B?enlCYkx6THB0aGhHYXlzTFY3MnZuSldNSGc2Umg1OFdOSVd5SkQyVTBpQUdp?=
 =?utf-8?B?VHhXa3FoS29SQlhZL3F1UDE5M0VxYXlLZkNHZlJmcnovby9FSndiaEdKd0M1?=
 =?utf-8?B?bkY4VVpCaUVLemdHOS9yL2I5RE1lTDZWbmIwK1dqaEpNVFVNZ1lLTmF2aG1x?=
 =?utf-8?B?dnVsbzNzVS9uMTA5ZjF2OWIvV3d2a05vN2lrYm9zNUUvTDMvZEprUzRaSDNG?=
 =?utf-8?B?ZmIxa3pXR3VWVEErak11YnNjd01aYnRrL2RFWEVYd3pVemdML2UreXhSVkxN?=
 =?utf-8?B?SVk1UmpGbUdiRnMyVFMwd2t1V1RCWFRjaFJnSjk3SDl2VEI2OURGcWxjY0pk?=
 =?utf-8?B?ZDR6SVhhSk0zclQ5Y3pJMUxuWkVrNkNveEhwRGpONmdQenY3dEg2VXpIa2hN?=
 =?utf-8?B?SmhTTkNtMzZLUER1UzVoNXdJUFVGbGQ2dGdreWp1SDVaaVlzZGd4R01qV2NI?=
 =?utf-8?B?VVZlL29IRTBrR2E0RDVWR0Q5TXNORm9qdGVjZGMwaUwyaXRsc0pITXhLa2JU?=
 =?utf-8?B?TjQyckV0MWo3U2x0ckZrZ3RrYzh4OFhPa3RIRFZFd0JxQkJKTmhlS1hqVmYz?=
 =?utf-8?B?UzRkN0dZWXpLNStDbk1wendKZnczQ3paa3hvekZ3bTRKMmhqU3QyNG8zd1FF?=
 =?utf-8?B?NjBwcnY5ZCtqWHhwdGFiR0hSVkliUGdRSUlPSjY5MDlUYUlvY1FhQlZaQldC?=
 =?utf-8?B?Vk9YZDM3T3kwWlNJaUFPWFpuVEtQM1VpUmwxbTN6WnkwS2dGUElSQ0g2elZK?=
 =?utf-8?B?S3ZpcytsTjNtUmVBNC9kQ1MyMlVUY2xEaHF5dEIvV05SRlZyb2dOTVBwa2du?=
 =?utf-8?B?bVIxdWkrTnloQnptZ2ZGM0NnbG1mYUErd3hGbVVSbDBpc1J6Z0M1RkQwbi9G?=
 =?utf-8?B?YXFhMStKWllRd2lQWURDblJGQ0lKdktsU3N3L3pGVzVicTI5Yml4bWpVZlVv?=
 =?utf-8?B?RXE4V1BKMDRGWG9ObVMwTlB3SXNxTXg5YW4yTDZ0VmhZMjhLTGFMMlhTWFE3?=
 =?utf-8?B?Y1dIVy83YnYvM0Z1aFVvUityVVZycUtEVC8zWGI5L1ZvaVRPNVMrdUs3elZI?=
 =?utf-8?B?WWZtNHUvYkJZb0dSL0FFSS94cmd3eGJvYzRTSVVkYU1hU2g2RjBzZnZLQ01v?=
 =?utf-8?B?bmZhM3VxdEtFcVBySEhyK0orOGl3eURKM29HVUpMVU9aaHFGenZQbk54Z2Fy?=
 =?utf-8?B?VXJhbzJIeHJQNGg2L0hFMHk4K2RMQ0FKWVYyem1XZDlyZmU1eUxjVUdGNDZN?=
 =?utf-8?B?anlpVDVBNWZCWk9PL3RUQ1VhLzhDWlJ2VTJuUFhvQkF5ai9yMFVMZUFBY0wr?=
 =?utf-8?B?cmRVbnZ0V2VwNzJRRGtxWmhkMkRuRnRSMFVYNFdlUU9OZWpYSWRQWGQ0WjAz?=
 =?utf-8?B?cG9kMlBodXZGSnhhSFUvTnpZUVNSbVhqT3VBeldXVVYzMkwzRUEwMldwdEVr?=
 =?utf-8?B?akFZdE1ocytzQjluQUNyT3UxcVZKZTdsSmYwSU9hTmpIdDNPYmppKytLUjJW?=
 =?utf-8?B?YkRZZm5vUWVnSTRkeTdpOFdhbUQzOUdmem9vRjZ6VHRYOFRPV2RkSGVHdktx?=
 =?utf-8?B?ZWk1QXNUcXo5MTlDSFFsWmxHOXNpN3dQNzJvS1lKTVp3SHp0VkFscE1KSlhU?=
 =?utf-8?Q?Pnd679EZm/rC6yE9I/K3I3WK4?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2944c19e-226a-4857-18d6-08da84c3f585
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6588.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Aug 2022 04:57:20.2257
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: FRHefAh8dW2scetON5oZ2Pt5KtAsnPsYFH8YwUhtLPqugHTHbFyKQgWw2f0MbAd8xQKZDO7fXf3SYAJYT4a4+g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4825
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 22-Aug-22 10:22 PM, Peter Zijlstra wrote:
> On Tue, Aug 02, 2022 at 11:41:42AM +0530, Ravi Bangoria wrote:
> 
>>> @@ -12358,58 +12374,14 @@ SYSCALL_DEFINE5(perf_event_open,
>>>  			goto err_context;
>>>  	}
>>>  
>>> -	event_file = anon_inode_getfile("[perf_event]", &perf_fops, event, f_flags);
>>> -	if (IS_ERR(event_file)) {
>>> -		err = PTR_ERR(event_file);
>>> -		event_file = NULL;
>>> -		goto err_context;
>>> -	}
>>> -
>>> -	if (task) {
>>> -		err = down_read_interruptible(&task->signal->exec_update_lock);
>>> -		if (err)
>>> -			goto err_file;
>>> -
>>> -		/*
>>> -		 * We must hold exec_update_lock across this and any potential
>>> -		 * perf_install_in_context() call for this new event to
>>> -		 * serialize against exec() altering our credentials (and the
>>> -		 * perf_event_exit_task() that could imply).
>>> -		 */
>>> -		err = -EACCES;
>>> -		if (!perf_check_permission(&attr, task))
>>> -			goto err_cred;
>>> -	}
>>> -
>>> -	if (ctx->task == TASK_TOMBSTONE) {
>>> -		err = -ESRCH;
>>> -		goto err_locked;
>>> -	}
>>
>> I think we need to keep (ctx->task == TASK_TOMBSTONE) check?
> 
> I think so too; in fact the code I have still has it, perhaps it was
> there write before this patch?
> 
>>> -
>>>  	if (!perf_event_validate_size(event)) {
>>>  		err = -E2BIG;
>>> -		goto err_locked;
>>> -	}
>>> -
>>> -	if (!task) {
>>> -		/*
>>> -		 * Check if the @cpu we're creating an event for is online.
>>> -		 *
>>> -		 * We use the perf_cpu_context::ctx::mutex to serialize against
>>> -		 * the hotplug notifiers. See perf_event_{init,exit}_cpu().
>>> -		 */
>>> -		struct perf_cpu_context *cpuctx =
>>> -			container_of(ctx, struct perf_cpu_context, ctx);
>>> -
>>> -		if (!cpuctx->online) {
>>> -			err = -ENODEV;
>>> -			goto err_locked;
>>> -		}
>>> +		goto err_context;
>>
>> Why did you remove this hunk? We should confirm whether cpu is online or not
>> before creating event. No?
> 
> Idem.
> 
> Perhaps it is best if we look at the end result of all these patches
> combined and then I'll fold the lot if we're in agreement and then we
> can forget about these intermediate steps.

Let me accumulate all these changes, rebase to v6.0-rc2 and send RFC v3.

Thanks,
Ravi