Re: [PATCH v4 13/18] selftests/nolibc: prepare /tmp for tmpfs or ramfs

[Zhangjin Wu]

Hi, Willy

> Hi Zhangjin,
> 
> On Mon, Jul 10, 2023 at 01:06:00PM +0800, Zhangjin Wu wrote:
> > > On Sat, Jul 08, 2023 at 02:38:57AM +0800, Zhangjin Wu wrote:
[...]
> > Now, we use "/tmp" directly in vfprintf, and we use argv0 for chmod_exe and
> > chroot_exe, so, the only user of "/tmp" is vfprintf currently. In this case, it
> > is a simple normal writable directory to allow create tmp files there, so,
> > agree very much to only reserve the mkdir part:
> > 
> >     /* create /tmp if not there. Silently fail otherwise */
> >     mkdir("/tmp", 0755);
> 
> OK, then I'll do that.
>

Thanks.

> > Another consideration before is whether it is required to be consistent with
> > the normal Linux systems, let the "/tmp" directory mounted as tmpfs at most of
> > the time,
> 
> That's not what I'm seeing on most of the systems I'm having access to,
> where /tmp is on a plain file system (either / or link to /var/tmp but
> always on disk, likely due to the huge size of the stuff stored there
> that is rarely used and that should not eat memory).
> 
> > but "/tmp" means ramfs for CONFIG_TMPFS=n currently even mount it
> > explicitly (ramfs is a fallback of tmpfs in such case), so, this assumption of
> > "/tmp" means tmpfs is not true currently.
> >
> > What I'm worried about is people in the future assume "/tmp" as tmpfs at the
> > first glance and use the features only provided by TMPFS but not provided by
> > RAMFS (I'm not sure which one they will use). so, I even tried to create a
> > "/tmp/tmpfs" flag for TMPFS and "/tmp/ramfs" flag for RAMFS before, since there
> > is no user to explicitly prefer TMPFS to RAMFS currently, at last, I removed
> > these flags from the sent patchsets. Based on the same logic, The removal of
> > tmpfs mount is of course ok.
> 
> Indeed, and also, please keep in mind what the purpose of nolibc-test is:
> make sure that the syscalls wrappers we write do work as expected, in part
> by allowing us to compare against another libc to figure whether it's
> the libc, the test or the kernel that causes any difference. The rest is
> purely out of scope. Thus it's not this test's business to verify that a
> tmpfs is indeed present after trying to mount it under /tmp, however it's
> this test business to make sure that options passed to mount() do work as
> expected, and that when a writable area is needed for a test, a working
> one is assigned. Thus for the specific case you mention, we don't care.
> And I'd go further, there can and should be reasonable prerequisites to
> run this test.
>

Ok.

> > So, Willy, is it ok for you to remove that mount line with corresponding update
> > of the commit message (and the subject title), or require me to send a revision
> > for this patch?
> 
> No worries, I've modified it accordingly with the following commit message,
> just let me know if you want to change anything:
> 
>   commit 11fddb386bd663a554cc08c5950d9da2c87a7267 (HEAD)
>   Author: Zhangjin Wu <falcon@xxxxxxxxxxx>
>   Date:   Sat Jul 8 02:38:57 2023 +0800
> 
>     selftests/nolibc: prepare /tmp for tests that need to write
>     
>     create a /tmp directory. If it succeeds, the directory is writable,
>     which is normally the case when booted from an initramfs anyway.
>     
>     This will be used instead of procfs for some tests.
>     
>     Reviewed-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
>     Signed-off-by: Zhangjin Wu <falcon@xxxxxxxxxxx>
>     Link: https://lore.kernel.org/lkml/20230710050600.9697-1-falcon@xxxxxxxxxxx/
>     [wt: removed the unneeded mount() call]
>     Signed-off-by: Willy Tarreau <w@xxxxxx>
>

Perfectly, Thanks a lot.

Best regards,
Zhangjin

> Thanks!
> Willy