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=-5.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 E4296C433E9 for ; Tue, 16 Mar 2021 01:18:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BEEBD64FA6 for ; Tue, 16 Mar 2021 01:18:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230397AbhCPBSH (ORCPT ); Mon, 15 Mar 2021 21:18:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234278AbhCPBRq (ORCPT ); Mon, 15 Mar 2021 21:17:46 -0400 Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 750D1C06174A for ; Mon, 15 Mar 2021 18:17:46 -0700 (PDT) Received: by mail-pg1-x52c.google.com with SMTP id o16so4548012pgu.3 for ; Mon, 15 Mar 2021 18:17:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:message-id:from:to:cc:subject:in-reply-to:references :user-agent:mime-version; bh=Sph6AbO38eZpjPHbAVWcmHuYYjTcu+vxy91BdosdlVM=; b=VUFP4FrPW+k4EN2pQyEmAs8weLEv/Sc1JXEOaOk5zlhjoSOkS+JzKYxL2FksFHCqRU d00qNcL5O6+8mH3kP6XVcn4mv8Xdy3rlfpkC/zc94q9r/1sdAa3yHgTOMCeaVULt2EiR yxjay1lIow3jUY5yGiZSrZmahUt1SK9MhWAQEF/u/4Ezmrc0Uw3tZ5SA5A+ecmCuOJQu TNqyIB4PorEsM2zA25LO1s0rDl6ewLn6kuxlt5q6D9mu0jUqIxWjTAN/gA7ZC2diDOSB iXSl01mUHVXF1ysyetpLYhLrX8koes2PnIGJWk1Ch8l/2rVPqExFIsOJT18agVVUtIBL 0qnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:from:to:cc:subject:in-reply-to :references:user-agent:mime-version; bh=Sph6AbO38eZpjPHbAVWcmHuYYjTcu+vxy91BdosdlVM=; b=K8h3uImwW40lcEcEJlZ9lCacuW9Dw90VE5YkFUeehouY+QtE2UUXKB06FpsBbmi1SD dEGsK9Tgr71u/jKO5ZdBk8XD8qlxsmxioVu+yBPNUqqnDVthhQrQbex7zbeGAmLMMowR kshSwPmcdYRGB60KAFPhXtxGBBd3AIiJAeTbhRaq7ORnLuCsXHyu2xzkdjX9FkYuZcuV 1n/PDfVdmV3zVrO3YbeisPEY6DUebjGuLnojvAYDo1MaqVKCwR21RgcE7j2aX3pi8Ksr 5JPyDjrct5ySJ+ewpg+eP/CG5NHeoXpYuPXDGob8/+5i7E+xD2b8Mlh2a7PVD3ZfjMXs cAPA== X-Gm-Message-State: AOAM530BtSo55pBFAAvyZzIU9KceevqCMb0OpITasBicl63bHTgHsCFM /aMF1ekUu85WkYt3slx570M= X-Google-Smtp-Source: ABdhPJzSg8KS/xvqQB+EYb+ystSg4SNroUVAhKf5YQ9JtKbG7dDSI6nwRwKdbcIY6hmEf0i4TxbYkA== X-Received: by 2002:a62:aa0a:0:b029:1ef:fe5:b172 with SMTP id e10-20020a62aa0a0000b02901ef0fe5b172mr12649748pff.9.1615857465909; Mon, 15 Mar 2021 18:17:45 -0700 (PDT) Received: from sun.local.gmail.com (219x123x138x129.ap219.ftth.ucom.ne.jp. [219.123.138.129]) by smtp.gmail.com with ESMTPSA id a20sm15442708pfl.97.2021.03.15.18.17.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Mar 2021 18:17:45 -0700 (PDT) Date: Tue, 16 Mar 2021 10:17:44 +0900 Message-ID: From: Hajime Tazaki To: johannes@sipsolutions.net Cc: linux-um@lists.infradead.org, jdike@addtoit.com, richard@nod.at, anton.ivanov@cambridgegreys.com, tavi.purdila@gmail.com, linux-kernel-library@freelists.org, linux-arch@vger.kernel.org, retrage01@gmail.com Subject: Re: [RFC v8 06/20] um: add UML library mode In-Reply-To: References: <30b989750979c00b5ceec4fe5faa6fb8332212d5.1611103406.git.thehajime@gmail.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.1 Mule/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-arch@vger.kernel.org On Mon, 15 Mar 2021 01:49:19 +0900, Johannes Berg wrote: > > On Wed, 2021-01-20 at 11:27 +0900, Hajime Tazaki wrote: > > > > +++ b/arch/um/lkl/include/asm/atomic.h > > @@ -0,0 +1,11 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_ATOMIC_H > > +#define __UM_LIBMODE_ATOMIC_H > > + > > +#include > > + > > +#ifndef CONFIG_GENERIC_ATOMIC64 > > +#include "atomic64.h" > > +#endif /* !CONFIG_GENERIC_ATOMIC64 */ > > That's not actually configurable, is it? When is this on or off? This is off when CONFIG_64BIT is off, because in that case (!64BIT) we use generic atomic64 facility of in-kernel. > > +static inline void cpu_relax(void) > > +{ > > + unsigned long flags; > > + > > + /* since this is usually called in a tight loop waiting for some > > + * external condition (e.g. jiffies) lets run interrupts now to allow > > + * the external condition to propagate > > + */ > > + local_irq_save(flags); > > + local_irq_restore(flags); > > Hmm? > > If interrupts are enabled, then you'll get them anyway, and if not, then > this does nothing? You might be right; the original LKL does yield-like operation if this function is called, but this might not be the case after the integration with UML. I will investigate this and fix it. > > +static inline void arch_copy_thread(struct arch_thread *from, > > + struct arch_thread *to) > > +{ > > + panic("unimplemented %s: fork isn't supported yet", __func__); > > +} > > "yet" seems kind of misleading - I mean, it really *won't* be, since > that's basically what UML is, no? > > I mean, in principle, nothing actually stops you from building a > linux.so instead of linux binary, and having main() renamed to > linux_main() so the application can start up whenever it needs to, or > something like that? I commented this in the reply-thread of the [00/20] patch. > > diff --git a/arch/um/lkl/include/uapi/asm/bitsperlong.h b/arch/um/lkl/include/uapi/asm/bitsperlong.h > > new file mode 100644 > > index 000000000000..f6657ba0ff9b > > --- /dev/null > > +++ b/arch/um/lkl/include/uapi/asm/bitsperlong.h > > @@ -0,0 +1,13 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_UAPI_BITSPERLONG_H > > +#define __UM_LIBMODE_UAPI_BITSPERLONG_H > > + > > +#ifdef CONFIG_64BIT > > +#define __BITS_PER_LONG 64 > > +#else > > +#define __BITS_PER_LONG 32 > > +#endif > > + > > +#include > > First, that include does nothing. We followed the way of arch/x86 (and other archs) for this. This actually includes include/asm-generic/bitsperlong.h, which does something I think. > Second, the comment there says: > > /* > * There seems to be no way of detecting this automatically from user > * space, so 64 bit architectures should override this in their > * bitsperlong.h. In particular, an architecture that supports > * both 32 and 64 bit user space must not rely on CONFIG_64BIT > * to decide it, but rather check a compiler provided macro. > */ > > > So you're doing exactly what it says *not* to? > > In fact, that totally makes sense - I mean, this is *uapi* and > applications don't have access to CONFIG_ defines etc. Sorry, this patch is wrong. There is additional diff in [10/20], which should be in the same patch here, which eliminates not to rely on CONFIG_*. I will fix this. > Do you expose that somehow to LKL users that makes this OK-ish? But it's > still very confusing, and you've not made sure the necessary header is > actually included. IIUC, lkl builds a (shared?) library, so it won't > really work this way? > > > +++ b/arch/um/lkl/include/uapi/asm/byteorder.h > > @@ -0,0 +1,11 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_UAPI_BYTEORDER_H > > +#define __UM_LIBMODE_UAPI_BYTEORDER_H > > + > > +#if defined(CONFIG_BIG_ENDIAN) > > +#include > > Same here. Same problem here; we should not split this in the patch and [10/20]. Will fix this too. > > +++ b/arch/um/lkl/um/delay.c > > @@ -0,0 +1,31 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include > > +#include > > +#include > > + > > +void __ndelay(unsigned long nsecs) > > +{ > > + long long start = os_nsecs(); > > + > > + while (os_nsecs() < start + nsecs) > > + ; > > At least do something like cpu_relax()? Although ... you made that do > nothing, so you probably want an arch-specific thing here? I think this isn't efficient at all, but portable enough, which is (sub)arch-specific requirement. -- Hajime From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52e.google.com ([2607:f8b0:4864:20::52e]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lLyLD-00HB0e-HN for linux-um@lists.infradead.org; Tue, 16 Mar 2021 01:17:51 +0000 Received: by mail-pg1-x52e.google.com with SMTP id n9so20568615pgi.7 for ; Mon, 15 Mar 2021 18:17:47 -0700 (PDT) Date: Tue, 16 Mar 2021 10:17:44 +0900 Message-ID: From: Hajime Tazaki Subject: Re: [RFC v8 06/20] um: add UML library mode In-Reply-To: References: <30b989750979c00b5ceec4fe5faa6fb8332212d5.1611103406.git.thehajime@gmail.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") 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-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: johannes@sipsolutions.net Cc: linux-um@lists.infradead.org, jdike@addtoit.com, richard@nod.at, anton.ivanov@cambridgegreys.com, tavi.purdila@gmail.com, linux-kernel-library@freelists.org, linux-arch@vger.kernel.org, retrage01@gmail.com On Mon, 15 Mar 2021 01:49:19 +0900, Johannes Berg wrote: > > On Wed, 2021-01-20 at 11:27 +0900, Hajime Tazaki wrote: > > > > +++ b/arch/um/lkl/include/asm/atomic.h > > @@ -0,0 +1,11 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_ATOMIC_H > > +#define __UM_LIBMODE_ATOMIC_H > > + > > +#include > > + > > +#ifndef CONFIG_GENERIC_ATOMIC64 > > +#include "atomic64.h" > > +#endif /* !CONFIG_GENERIC_ATOMIC64 */ > > That's not actually configurable, is it? When is this on or off? This is off when CONFIG_64BIT is off, because in that case (!64BIT) we use generic atomic64 facility of in-kernel. > > +static inline void cpu_relax(void) > > +{ > > + unsigned long flags; > > + > > + /* since this is usually called in a tight loop waiting for some > > + * external condition (e.g. jiffies) lets run interrupts now to allow > > + * the external condition to propagate > > + */ > > + local_irq_save(flags); > > + local_irq_restore(flags); > > Hmm? > > If interrupts are enabled, then you'll get them anyway, and if not, then > this does nothing? You might be right; the original LKL does yield-like operation if this function is called, but this might not be the case after the integration with UML. I will investigate this and fix it. > > +static inline void arch_copy_thread(struct arch_thread *from, > > + struct arch_thread *to) > > +{ > > + panic("unimplemented %s: fork isn't supported yet", __func__); > > +} > > "yet" seems kind of misleading - I mean, it really *won't* be, since > that's basically what UML is, no? > > I mean, in principle, nothing actually stops you from building a > linux.so instead of linux binary, and having main() renamed to > linux_main() so the application can start up whenever it needs to, or > something like that? I commented this in the reply-thread of the [00/20] patch. > > diff --git a/arch/um/lkl/include/uapi/asm/bitsperlong.h b/arch/um/lkl/include/uapi/asm/bitsperlong.h > > new file mode 100644 > > index 000000000000..f6657ba0ff9b > > --- /dev/null > > +++ b/arch/um/lkl/include/uapi/asm/bitsperlong.h > > @@ -0,0 +1,13 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_UAPI_BITSPERLONG_H > > +#define __UM_LIBMODE_UAPI_BITSPERLONG_H > > + > > +#ifdef CONFIG_64BIT > > +#define __BITS_PER_LONG 64 > > +#else > > +#define __BITS_PER_LONG 32 > > +#endif > > + > > +#include > > First, that include does nothing. We followed the way of arch/x86 (and other archs) for this. This actually includes include/asm-generic/bitsperlong.h, which does something I think. > Second, the comment there says: > > /* > * There seems to be no way of detecting this automatically from user > * space, so 64 bit architectures should override this in their > * bitsperlong.h. In particular, an architecture that supports > * both 32 and 64 bit user space must not rely on CONFIG_64BIT > * to decide it, but rather check a compiler provided macro. > */ > > > So you're doing exactly what it says *not* to? > > In fact, that totally makes sense - I mean, this is *uapi* and > applications don't have access to CONFIG_ defines etc. Sorry, this patch is wrong. There is additional diff in [10/20], which should be in the same patch here, which eliminates not to rely on CONFIG_*. I will fix this. > Do you expose that somehow to LKL users that makes this OK-ish? But it's > still very confusing, and you've not made sure the necessary header is > actually included. IIUC, lkl builds a (shared?) library, so it won't > really work this way? > > > +++ b/arch/um/lkl/include/uapi/asm/byteorder.h > > @@ -0,0 +1,11 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __UM_LIBMODE_UAPI_BYTEORDER_H > > +#define __UM_LIBMODE_UAPI_BYTEORDER_H > > + > > +#if defined(CONFIG_BIG_ENDIAN) > > +#include > > Same here. Same problem here; we should not split this in the patch and [10/20]. Will fix this too. > > +++ b/arch/um/lkl/um/delay.c > > @@ -0,0 +1,31 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include > > +#include > > +#include > > + > > +void __ndelay(unsigned long nsecs) > > +{ > > + long long start = os_nsecs(); > > + > > + while (os_nsecs() < start + nsecs) > > + ; > > At least do something like cpu_relax()? Although ... you made that do > nothing, so you probably want an arch-specific thing here? I think this isn't efficient at all, but portable enough, which is (sub)arch-specific requirement. -- Hajime _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um