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: <50c82ae4-fb71-c534-d285-5991baa37150@intel.com>
Date:   Mon, 7 Nov 2022 14:40:52 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.4.1
Subject: Re: [PATCH v3 4/5] selftests/resctrl: Cleanup properly when an error
 occurs in CAT test
Content-Language: en-US
To:     Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Shuah Khan <shuah@kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
References: <20221101094341.3383073-1-tan.shaopeng@jp.fujitsu.com>
 <20221101094341.3383073-5-tan.shaopeng@jp.fujitsu.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20221101094341.3383073-5-tan.shaopeng@jp.fujitsu.com>
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?YkxoWnFFK2dXT3d4clBSRnVRYkV4SDFjY1YwQkUvRVJWMHkzWW1LejlrTlpO?=
 =?utf-8?B?dElBR0g2THczeTI0MTd5Z3BiWk5ZK2N6TDlVWWVGeVZvUlZZVDdwSlRVZ2Qz?=
 =?utf-8?B?VkxOazM1WTlxWVgrb2lSNEh2NXNvTE9jd3lIKy82RkZkSVdBdklYSE9NWlpF?=
 =?utf-8?B?bWdScWtGTC9BQTVFeXAxeS9UNlgwYmUvTUNMczRxc2pyR2lHRmVucEpEczY0?=
 =?utf-8?B?VVhtYTI5TFlsMlNVRDUwN2FnQ0hBVEt2aGxpWHhIZUxUd1gwa1dPTm1wUFR4?=
 =?utf-8?B?N0FULzZqUVhmNFJjTG9kdG1NZ2ZZM3U2cHhkOVhLdmllcTdnUG1BbDB2eFpY?=
 =?utf-8?B?UFdIWHpTZGpzWDVHa0RyMXY2cEtLV2dMWHYzR3dQVnNBWSsyRXlWcFQ4Q3Jx?=
 =?utf-8?B?ekE5Tm5xanJiNUxMMUxtaVI5SlhMZlRkRUxSS3paZVNOS1J0WVAzSnBZQUF1?=
 =?utf-8?B?QjFrQTVraXBqRGFlaVRHNCtiT2I3MXZaMHV3VFgxTE05T2ZRUVU0NTg3VXZO?=
 =?utf-8?B?UEZEblZ0RTBxVEh5cDdZaXNpWXRrV21MZVgvZ3pJYkg3VUc1ZHZjTngvZXFR?=
 =?utf-8?B?c1kzTEFGdGFLVW55Rzg1Zk5LQWFGZGljWUJWUXFZc1JPYWc4RFR2S256cEdC?=
 =?utf-8?B?NGl1YWdteWlSWVlHbzBWdEhlenEybFhkU2lSNnZraTNXL2Z6ZUJsU2VYTGEy?=
 =?utf-8?B?QWNVaDBNdmN2QytCK2ZpU1oxT1dZTVNSWmFjRDZWK2RYbFJkbXFKclFrNFVa?=
 =?utf-8?B?cWFHNWgvT0gvdGtqREtmb3RVY1FNZWloZEtVWStjNVlVMjl2aGQ2R2pydm1C?=
 =?utf-8?B?TldnOEI5bFh5RUdFQjVoWUpmVkg3bHZta0pSMmZMVmg1RjVXSVRMLzAwVGJp?=
 =?utf-8?B?V1JpTnBFcmFpK0VPVzFDbHFsL1JmSEtleloyUjFZMU9ZdFpSVnFDdk52STVz?=
 =?utf-8?B?MTJCeVJFQkVRM2c5cGJaUWlXNHV5M1pVV08yN1RjSHBmcFNmZ2hncW5yWURk?=
 =?utf-8?B?SlVBc0lENnZTRVFoR2MvZlhVOGlLZmlJb0FvZ1VHQUNtS0xDdEhXVVFYdk91?=
 =?utf-8?B?bGZNUXlZWUVzbjEwT3I4bW40NktYWlBkTkttTlFJQWdnL2I5U0xBY1lXV3dm?=
 =?utf-8?B?SFE1Ym1OMjJ3UXU0WUdTclMyeUpCeW1sdGFtb3JTVjdjcDJxNU5GRmhtT1Qr?=
 =?utf-8?B?WGp6NUdWTHI4cHhId0dGbkdXRXN4UnMyaitjemtpM1ZZTTlKMlRJazhNc3VB?=
 =?utf-8?B?amlmRk9RTFF1WFlEV09Nb1RCK2RZT0tncUNMTzE2alhwUEl4eWVrZE5JZytF?=
 =?utf-8?B?ODlCaHJlYytwS1lhSTlpSnpBMlUwYjBlcm55VERWMlRyNnhtZ2NxZktMSFBz?=
 =?utf-8?B?UlY2NTlQMXdsdTFUUzYzZXY1Z0hTbktUdE5XN3dsV0hSejdMNWVFM1FRcDBC?=
 =?utf-8?B?TkREZkttVVdmaXl6TU45WE9YMHkwaDdwWnRjMHhvSzF4bEhMRDdsenNaNm9l?=
 =?utf-8?B?NkNmUm1mVGoyUVJ1TFNaTWI3UVNtbGlzUG15d3lSQkxaZWFGaGlZWk9yc3pO?=
 =?utf-8?B?cTdZOHoxODBiVWdFNE96YWR4R2xOSzlkeFBFZTQrWDBxbWt1aXZMQksrVnl6?=
 =?utf-8?B?YmRuU3puancrRUo5U1l4bG5EVXM5RkFMaXB6YnhIdWNXbGxrRE5DWTJZTXJw?=
 =?utf-8?B?QjNMVGN4RWVxem13T21FOE1LdnRxMkMwUFJDUVpvQWZmdkRsNVlDVTl3bW9y?=
 =?utf-8?B?bDBBdXlrdWRyNlNwRnp0STBqck9xMGwyUnhUdlJuZUhCZGZ3RHVWVXhQV0tV?=
 =?utf-8?B?YXVOU0FXU1M4THRReFhzMTYvRG44MUtQWVdwS0NzQmM4Ulh3eDUxRTA5bXZv?=
 =?utf-8?B?eUZmcHdBVWQ2VTRTaU1ta0QxRTIwSjFGTXhWV0xocjVURWlTaFFaRkJ0NUo3?=
 =?utf-8?B?bGIwOENYdkJibk95R2lEOExpaC9XMG0vem5tRGM4VExNVW03TVZkeG1LUzB0?=
 =?utf-8?B?dWRldTZYdjcxL2ZMempZSEllT3RMRGFZeGhSNWYrOTNQRWFTY1RkMWtvT1A3?=
 =?utf-8?B?ekt5dk5SYngyQlcwQ0JtMUFYRU9ZV0dOSGhZTVdZZnZmb01HalFUeExvWVdI?=
 =?utf-8?B?OHJNM2Mza3pMQThHWElub3p4Wk1lKy9kNU1FL1doRElncENvVGhYRlBnREp2?=
 =?utf-8?B?Mnc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 19bd9189-bd88-4595-6b0f-08dac1112171
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Nov 2022 22:40:54.7282
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: rLKgOUTElc7XRJFQzNe4c2Rv0FcbajjuH1HJeBLypj2C5Uy+/cLTAk4DQOmjhzECV8ucc4buTJQR0pd35fpDtCkPL0EI8IPUJ2lPhr7IWv8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5344
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE 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

Hi Shaopeng,

On 11/1/2022 2:43 AM, Shaopeng Tan wrote:
> After creating a child process with fork() in CAT test, if there is
> an error occurs or such as a SIGINT signal is received, the parent

I find the above hard to read. How about "..., if an error occurs or
a signal such as SIGINT is received, ..."

> process will be terminated immediately, but the child process will not
> be killed and also umount_resctrlfs() will not be called.
> 
> Add a signal handler like other tests to kill child process, umount
> resctrlfs, cleanup result files, etc. when an error occurs.
> 
> Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> ---
>  tools/testing/selftests/resctrl/cat_test.c | 28 +++++++++++++++-------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 

...

> @@ -201,7 +212,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>  		    sizeof(pipe_message)) {
>  			close(pipefd[1]);
>  			perror("# failed signaling parent process");
> -			return errno;

It looks like pipefd[1] will be closed twice if the write() failed.

It does look strange to let the child continue to its infinite loop
after the write() failed. I assume that it is because the parent will
also be stuck and the new ctrl_handler() is expected to unblock both?
Could you please add a comment to the code to clarify this flow?

Thank you very much

Reinette