From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7965C4338F for ; Mon, 2 Aug 2021 15:39:41 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A8326610FB for ; Mon, 2 Aug 2021 15:39:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A8326610FB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7oTjs5u20U/RfjhYwxXWIeXJvJADuQPeZS3ZCG63QAI=; b=Fy5frRr3mD+liY /2IggX3gzXW8jYBFIvp+IXC3pvDhZgCp92ak8Aerk4OGxGF3TYCiwJdApT27gsu3g6ZaSolgD54VH eQjPt41nrlasFWJOdRjTIUdQ37Xr+3vMOhSK882EFMhOQ+jyu3KkAJt1ooS1NQm+tpr/zAMMC+ZnF L+nGUe8oUFIAWXmKVrr0cIfP9qysnzLqDG7KccaMP8y/vEbu+8PE8nxZLtq4geWxawYtFOTtIo4oJ MUk2+4f9mR5RCF/rlN/RpZW4T8XQhPWAwFk05CJxMHALg1L5O4vgtJyFs61DjFiW1p5timn2f9zSM HfO2VfRoe/aKNyR72VrQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAa0e-00Gze2-W9; Mon, 02 Aug 2021 15:37:45 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAa0a-00Gzd5-LO for linux-arm-kernel@lists.infradead.org; Mon, 02 Aug 2021 15:37:42 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C144911D4; Mon, 2 Aug 2021 08:37:38 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DA19F3F66F; Mon, 2 Aug 2021 08:37:37 -0700 (PDT) Date: Mon, 2 Aug 2021 16:36:15 +0100 From: Dave Martin To: Mark Brown Cc: Catalin Marinas , Will Deacon , Shuah Khan , linux-arm-kernel@lists.infradead.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v3 3/4] kselftest/arm64: Add tests for SVE vector configuration Message-ID: <20210802153614.GC25258@arm.com> References: <20210729151518.46388-1-broonie@kernel.org> <20210729151518.46388-4-broonie@kernel.org> <20210729160642.GP1724@arm.com> <20210729173411.GT4670@sirena.org.uk> <20210802102517.GA25258@arm.com> <20210802113330.GD4668@sirena.org.uk> <20210802123749.GB25258@arm.com> <20210802141939.GF4668@sirena.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210802141939.GF4668@sirena.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210802_083740_837484_9E8760B6 X-CRM114-Status: GOOD ( 31.69 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Aug 02, 2021 at 03:19:39PM +0100, Mark Brown wrote: > On Mon, Aug 02, 2021 at 01:37:50PM +0100, Dave Martin wrote: > > On Mon, Aug 02, 2021 at 12:33:30PM +0100, Mark Brown wrote: > > > > That really doesn't seem like a good idea - it's just asking for > > > fragility if a signal gets delivered to the parent process or something. > > > Even if almost all the time there will only be one trip through the loop > > > we should still have the loop there for those few cases where it > > > triggers. > > > This concern only applies when the program actually registers signal > > handlers. > > > wait() can't return for any other reason, and it mustn't, precisely > > because historically software would have made this assumption. This is > > one reason why wait3() etc. are separate functions. > > That's great for the reader with a detailed knowledge of exactly what > error handling can be skipped and how standards conforming Linux is but > less good for the reader who is merely aware of best practices. I am > not clear what the problem that is solved by removing the loop here is > TBH - to me it just makes it less obvious that we've handled everything. Ok, leave it as is then. (It would be good to collect some best-practice guidance on how to actually use syscalls, but that's clearly way out of scope here...) > > That aside though, can't we use popen(3)? > > > I tend to forget about popen because it is "boring" to use it, but it > > looks like it fits this case quite well. Then it would be libc's > > problem how to fork and wait safely. > > popen() appears to be break the _SET_VL_ONEXEC test. Between a lack of > strace in my test filesystem and not spotting anything obvious in the > glibc sources I can't tell exactly where it's doing something different, > though it does feel like it should be a separate testcase if it's > anything interesting. I do think there is value in having exactly > what's done to start the child process be clear in the test program, and > that coverage of anything interesting from popen() could be done > incrementally. Ah, dang, popen() will run the target program via a shell, so there will actually be two fork-exec()s, with the VL being reset to default by the second exec. Using PR_SET_SET_VL with popen() still makes sense, but if you want the target program to get the new VL (not just the shell) then you'd need PR_SVE_VL_INHERIT. Then we would get confused later when trying to test the !PR_SVE_VL_INHERIT case. The way to "fix" this would be to have the shell invoke something like vlset, but that will blur the test in a different way, adding even more confusion. So Ack, we can't test all the variations using the popen() method, so we probably shouldn't use it here at all. This is the kind of reason why I tend not to go for it, I guess -- it looks convenient, but it's just that little bit overcooked as an API. *sigh* I'll review your final version of the series, but I guess we're all good. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel