From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22b.google.com (mail-oi1-x22b.google.com [IPv6:2607:f8b0:4864:20::22b]) by sourceware.org (Postfix) with ESMTPS id 987AB3858D28 for ; Sun, 23 Jul 2023 19:10:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 987AB3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oi1-x22b.google.com with SMTP id 5614622812f47-3a3efee1d44so2750040b6e.3 for ; Sun, 23 Jul 2023 12:10:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690139444; x=1690744244; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=uQUQt1Gw4xA2KLJ6xaUP5vec3SIdjR23uHIPzOowA2E=; b=HGK0AIOa6vCCRmJk+tE3yPHlXjOXtrTMhtXFaRvH9SxxA0Z9UOTQFq3ct/zUJNsq2D 5IjZjPVOXwZdM4awiHOs++aBshTFVYQhkEp6/e01OVn3KEFAOokY1ul+2M+9DJif7wIA dr0INc/MPBj/M45/J3XcoUwF1ZOJoFUELIiHUE07yhdjJZfS221zZsAZwzLHmqZl33i2 9U6Lo6JygPPmTbPLlB+DKYlHpGybe+WeUh622tRyTmQByrNr6+1PDJ53UiJhNYzIUc2r 1hAoiswNFa9uvnII5g5n7HwD/ZUxuiRF37abmQaA0ey4TXvXieTaXIQ6mKW0D3M5bzVk XwkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690139444; x=1690744244; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uQUQt1Gw4xA2KLJ6xaUP5vec3SIdjR23uHIPzOowA2E=; b=KzH4xHh9OkyOspbKtziE7LCnB4Dcaf0/zWeZ9TrV/XJIgnW4vQZRq9o2BszgIN6reL sDa0TFGE5p5SoZBx00z6YRNnvlddrKPY2EH+859ZqvonWG9li7OQ7Mya9yCFRJteNeS1 tYYOqMXu/zesCUG2lxyAPb6ImByTK3zwpb46numgx+SG2wYlCiAVvVlqEd3Mtq7AtGMC JGue8RNIOLB61QZVdIYFAlHDYvhNR/kd8oRCVVnMP5eQ9d9+D5q9YtOMj4ioya4WxScL MnQuVNS7VZNensfc+RSclNrZXEYaoW/v9QxriRL/ewzacesDSCTiqgzYWilb97NNrIQ4 fy5w== X-Gm-Message-State: ABy/qLaf2iQVo6tZmxIdtMXf4mExCqvX4QtUq0pblcK7r5bh6ej3yEuh QZYqMoprBTS6FERiW/XDnzTECp6eF9E1eDi2sUl+XQ== X-Google-Smtp-Source: APBJJlGbfz3sGPr+QQ+bTQnaHURTMQuy8B9i3TwYregqCMTvQy/pGJ5aJ0nkObYbTB0icU5B4gx+8w== X-Received: by 2002:aca:1301:0:b0:3a4:357a:ebf4 with SMTP id e1-20020aca1301000000b003a4357aebf4mr7841560oii.0.1690139444623; Sun, 23 Jul 2023 12:10:44 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c0:d4d2:c40b:9f28:ce1c:2111? ([2804:1b3:a7c0:d4d2:c40b:9f28:ce1c:2111]) by smtp.gmail.com with ESMTPSA id w11-20020a0568080d4b00b003a43759b9cdsm3388768oik.29.2023.07.23.12.10.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 23 Jul 2023 12:10:43 -0700 (PDT) Message-ID: <8f9cfc43-beb1-97ce-d731-8b0fe6fcc983@linaro.org> Date: Sun, 23 Jul 2023 16:10:40 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v3 2/3] linux: statvfs: allocate spare for f_type Content-Language: en-US To: =?UTF-8?B?0L3QsNCx?= Cc: Florian Weimer , libc-alpha@sourceware.org References: <662dabdece8e902a1234b84cdc46ffefb107397d.1688396342.git.nabijaczleweli@nabijaczleweli.xyz> <4f0ff835-ef3d-5739-6d6d-1a360d97fd68@linaro.org> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 21/07/23 12:34, наб wrote: > On Fri, Jul 21, 2023 at 10:03:05AM -0300, Adhemerval Zanella Netto wrote: >> On 03/07/23 11:59, наб via Libc-alpha wrote: >>> diff --git a/sysdeps/unix/sysv/linux/bits/statvfs.h b/sysdeps/unix/sysv/linux/bits/statvfs.h >>> index 8dfb5ce761..cf98460e00 100644 >>> --- a/sysdeps/unix/sysv/linux/bits/statvfs.h >>> +++ b/sysdeps/unix/sysv/linux/bits/statvfs.h >>> @@ -51,7 +51,8 @@ struct statvfs >>> #endif >>> unsigned long int f_flag; >>> unsigned long int f_namemax; >>> - int __f_spare[6]; >>> + unsigned int f_type; >>> + int __f_spare[5]; >>> }; >>> >>> #ifdef __USE_LARGEFILE64 >>> @@ -71,7 +72,8 @@ struct statvfs64 >>> #endif >>> unsigned long int f_flag; >>> unsigned long int f_namemax; >>> - int __f_spare[6]; >>> + unsigned int f_type; >>> + int __f_spare[5]; >>> }; >>> #endif >>> >> All ABIs define fstatfs/statfs64 as signed, as the constants in include/uapi/linux/magic.h. >> Should we do the same here? > Kernel sources say > include/asm-generic/compat.h: compat_int_t f_type; > include/linux/statfs.h: long f_type; > include/uapi/asm-generic/statfs.h: __statfs_word f_type; > include/uapi/asm-generic/statfs.h: __statfs_word f_type; > include/uapi/asm-generic/statfs.h: __u32 f_type; > and > arch/alpha/include/uapi/asm/statfs.h:#define __statfs_word __u32 > arch/parisc/include/uapi/asm/statfs.h:#define __statfs_word long > include/uapi/asm-generic/statfs.h:#ifndef __statfs_word > include/uapi/asm-generic/statfs.h:#define __statfs_word __kernel_long_t > include/uapi/asm-generic/statfs.h:#define __statfs_word __u32 > so it's a damn mess: it looks to me like it's a signed long when it just > kinda worked out that way but u32 when actually constructing this ABI > for new arches(?). >From kernel headers it is 'long' for 64 bits and 'uint32_t' for 32 bits (and off course with a couple of architectures exceptions). > > I opted for an u32 because the raw constants > represent "4 bytes of data", not "4-byte number"; > this is also consistent with the hurd. > > That said, > $ man fstatfs | grep -i "$(printf '%08x\n' $(man fstatfs | grep -o '0x[^ ]*') | grep '^[8-f]')" > BPF_FS_MAGIC 0xcafe4a11 > BTRFS_SUPER_MAGIC 0x9123683e > CIFS_MAGIC_NUMBER 0xff534d42 > EFIVARFS_MAGIC 0xde5e81e4 > F2FS_SUPER_MAGIC 0xf2f52010 > HPFS_SUPER_MAGIC 0xf995e849 > HUGETLBFS_MAGIC 0x958458f6 > RAMFS_MAGIC 0x858458f6 > SELINUX_MAGIC 0xf97cff8c > SMB2_MAGIC_NUMBER 0xfe534d42 > VXFS_SUPER_MAGIC 0xa501fcf5 > XENFS_SUPER_MAGIC 0xabba1974 > so a quick test with > $ cat fs.cpp > #include > #include > #include > #include > > int main() { > printf("signed? %d\n", std::is_signed_v); > printf("sizeof? %zu\n", sizeof(statfs::f_type)); > struct statfs sf; > fstatfs(0, &sf); > printf("sf.f_type as lu=%lu\n", sf.f_type); > unsigned int tpu = sf.f_type; > int tpd = sf.f_type; > printf("sf.f_type as d=%d? %d\n", tpu, tpu == HUGETLBFS_MAGIC); > printf("sf.f_type as u=%u? %d\n", tpd, tpd == HUGETLBFS_MAGIC); > } > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > amd64 > signed? 1 > sizeof? 8 > sf.f_type as lu=2508478710 > sf.f_type as d=-1786488586? 1 > sf.f_type as u=2508478710? 1 > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > ppc64el > signed? 1 > sizeof? 8 > sf.f_type as lu=2508478710 > sf.f_type as d=-1786488586? 1 > sf.f_type as u=2508478710? 1 > > $ dpkg --print-architecture; ./fs < /dev/hugepages/ > s390x > signed? 0 > sizeof? 4 > sf.f_type as lu=2240043254 # my vm doesn't have hugetlbfs, so used ramfs > sf.f_type as d=-2054924042? 1 > sf.f_type as u=2240043254? 1 > reveals that there aren't really any issues we run into with signedness here. > In C: > switch(tpu/tpd/sf.f_type) { case ..._MAGIC: puts("tpu"); } works, > and comparing against const unsigned tt = ..._MAGIC; works; > comparing against const int tt = ..._MAGIC; doesn't work for sf.f_type. > > In C++: > c++ fs.cpp -o fs > fs.cpp:18:27: error: case value evaluates to 2508478710, which cannot be narrowed to type 'int' [-Wc++11-narrowing] > 18 | switch(tpd) { case HUGETLBFS_MAGIC: puts("tpd"); } > | ^ > /usr/include/linux/magic.h:19:26: note: expanded from macro 'HUGETLBFS_MAGIC' > 19 | #define HUGETLBFS_MAGIC 0x958458f6 /* some random number */ > | ^ > 1 error generated. > make: *** [: fs] Error 1 > > So this Must be an u32 to behave correctly in C++, > we don't lose anything else for making it unsigned, > and it's consistent with Hurd. > > Thoughts? Fair enough, I think with current kernel ABI status using unsigned would be the best option.