Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-156-g081acc5ed5-fm-20230206.001-g081acc5e
Mime-Version: 1.0
Message-Id: <1e7da4e7-392d-4a9e-aa95-0599a0c84419@app.fastmail.com>
In-Reply-To: <1af05c1441e9f96870be1cc20b1162e3f5043b2e.1675734681.git.chenfeiyang@loongson.cn>
References: <cover.1675734681.git.chenfeiyang@loongson.cn>
 <1af05c1441e9f96870be1cc20b1162e3f5043b2e.1675734681.git.chenfeiyang@loongson.cn>
Date:   Tue, 07 Feb 2023 15:30:58 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "chris.chenfeiyang" <chris.chenfeiyang@gmail.com>, w@1wt.eu,
        "Paul E. McKenney" <paulmck@kernel.org>
Cc:     "Feiyang Chen" <chenfeiyang@loongson.cn>,
        "Huacai Chen" <chenhuacai@kernel.org>,
        "Jiaxun Yang" <jiaxun.yang@flygoat.com>, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()
Content-Type: text/plain
Precedence: bulk

On Tue, Feb 7, 2023, at 03:09, chris.chenfeiyang@gmail.com wrote:
> From: Feiyang Chen <chenfeiyang@loongson.cn>
>
> Neither __NR_newfstatat nor __NR_stat is defined on new architecture
> like LoongArch, but we can use statx() to implement sys_stat().
>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>

This looks nice, it should also help on 32-bit architectures that
only have stat64 but not newfstatat or stat.

> +#if defined(__NR_newfstatat) || defined(__NR_stat)
>  static __attribute__((unused))
>  int sys_stat(const char *path, struct stat *buf)
...
> +#else
> +#error None of __NR_newfstatat, __NR_stat, nor __NR_statx defined, 
> cannot implement sys_stat()
> +#endif

Given that all architectures implement statx the same way, I wonder
if we can't just kill off the old function here and always use statx.

That would also allow removing the architecture specific
sys_stat_struct definitions in all arch-*.h files.

> +struct statx_timestamp {
> +	__s64	tv_sec;
> +	__u32	tv_nsec;
> +	__s32	__reserved;
> +};
> +
> +struct statx {
> +	/* 0x00 */
> +	__u32	stx_mask;	/* What results were written [uncond] */
> +	__u32	stx_blksize;	/* Preferred general I/O size [uncond] */
> +	__u64	stx_attributes;	/* Flags conveying information about the file 
> [uncond] */
> +	/* 0x10 */
> +	__u32	stx_nlink;	/* Number of hard links */
> +	__u32	stx_uid;	/* User ID of owner */
> +	__u32	stx_gid;	/* Group ID of owner */
> +	__u16	stx_mode;	/* File mode */
> +	__u16	__spare0[1];
> +	/* 0x20 */
> +	__u64	stx_ino;	/* Inode number */
> +	__u64	stx_size;	/* File size */
> +	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
> +	__u64	stx_attributes_mask; /* Mask to show what's supported in 
> stx_attributes */
> +	/* 0x40 */
> +	struct statx_timestamp	stx_atime;	/* Last access time */
> +	struct statx_timestamp	stx_btime;	/* File creation time */
> +	struct statx_timestamp	stx_ctime;	/* Last attribute change time */
> +	struct statx_timestamp	stx_mtime;	/* Last data modification time */
> +	/* 0x80 */
> +	__u32	stx_rdev_major;	/* Device ID of special file [if bdev/cdev] */
> +	__u32	stx_rdev_minor;
> +	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
> +	__u32	stx_dev_minor;
> +	/* 0x90 */
> +	__u64	stx_mnt_id;
> +	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
> +	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
> +	/* 0xa0 */
> +	__u64	__spare3[12];	/* Spare space for future expansion */
> +	/* 0x100 */
> +};

Can't we just #include <linux/stat.h> here to avoid having to maintain
a duplicate copy?

    Arnd