On 2023-06-30 05:23:35+0800, Zhangjin Wu wrote:
Hi Zhangjin,
On 2023-06-28 21:59:22+0800, Zhangjin Wu wrote:
I'm preparing a revision for this series, in the past days, when I was working on testing our new 'minimal' kernel config support for all of the architectures, the time cost (and wait) is really appreciable and the repeated develop and test is really a big pain, I can also image when you was working on stack-protector and Willy was working on lots of old features ;-)
To be honest I almost never built a kernel. Most of the time I tested my stuff with qemu-user. This made the dev-cycle really fast, especially with a binfmt setup that launches foreign binaries automatically with qemu-user.
Yeah, qemu-user-static + binfmt_misc work perfectly, but my host kernel is not that new, so, I'm still a little worried about that there may be some hidden issues.
qemu-user shouldn't have specific requirements for the host kernel. Could you elaborate?
As you explained before, I knew the idea of using '/proc/self' here is important to not using a fixed-time file, besides our proposed method (make sure it at least not fail, just skip for !procfs):
- CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break; + CASE_TEST(stat_timestamps); EXPECT_SYSZR(proc, test_stat_timestamps()); break;To further avoid skip it for !procfs (I don't mean relaly disable it for the default tinyconfig support, which need more discuss, at least provide the possibility to pass without procfs), do you like this change? it doesn't depend on 'proc' now.
- if (stat("/proc/self/", &st)) + if (stat("/proc/self/", &st) && stat("/init", &st) && stat("/", &st))The "/init" is compiled for 'run' target every time, so, the time stamp should be dynamic enough, for libc-test, the /proc/self should be always there (if still not enough, we can reuse the init file list here), the "/" here is only for the worst-case scene ;-)
Both aproaches seem fine. Just skipping on !proc seems good enough.
To get less skips, let's use the second method, just updated my local patches ;-)
As for enabling proc in the test configs I just tested a plain tinyconfig vs one with CONFIG_PROC_FS enabled:
tinyconfig: 375.06user 53.21system 2:05.80elapsed tinyconfig + CONFIG_PROC_FS: 397.77user 56.84system 2:09.24elapsed
The overhead seems acceptable.
Yeah, only one option is ok, but "multiple options x multiple architectures x multiple repeated runs", that is 'huge' ;-)
In your other patchset you mentioned a few options that were needed. But if we can drop CONFIG_NET completely, reduce CONFIG_TMPFS to CONFIG_MEMFD I would like to reevaluate the overall impact.
Note as for disabling memfd:
It seems currently MEMFD_CREATE is hardwired to only be enabled when either TMPFS or HUGETLBFS is enabled.
But the memfd code and syscalls seem to work perfectly fine with those options disabled. I'll send a patch to fix up the Kconfigs to enable that usecase.
Good catch!
but for the vfprintf test cases, It is able to open a file from tmpfs directly. If no tmpfs, use the default ramfs (initramfs uses) instead, this will also avoid the new flags trying (to silence the warning).
static int expect_vfprintf(int llen, size_t c, const char *expected, const char *fmt, ...) { + static const char *tmpfile = "/tmp/nolibc-vfprintf"; + struct stat stat_buf;
This should not be static.
int ret, fd, w, r; char buf[100]; FILE *memfile; va_list args; - fd = memfd_create("vfprintf", 0); + if (stat("/tmp/.", &stat_buf)) { + pad_spc(llen, 64, "[SKIPPED]\n"); + return 0; + }
Instead of checking with stat() here it would be nicer to check the result of open() below.
+ + fd = open(tmpfile, O_CREAT | O_TRUNC | O_RDWR, 0755);
Seems like a good usecase for open("/tmp", O_TMPFILE | O_EXCL | ...)
... + unlink(tmpfile);
... and drop this.
fd = memfd_open(); + if (fd == -1) + fd = open("/tmp", O_TMPFILE | O_EXCL | O_CREAT | O_RDWR, 0600); + if (fd == -1) + skip()
May be enough.
...tmpfs is mounted (in another patch) like procfs in prepare() for pid==1.
I plan to use this method in the revision, do you like this?
memfd_create() was designed to do this work, but in current stage, opening tmpfile ourselves may be better.
I'm not really a fan of having so much fallback code for features that should always be available. But if we can keep it straightforward it's probably fine. Or Willy has another opinion :-)