MIME-Version: 1.0
References: <cover.1675734681.git.chenfeiyang@loongson.cn> <1af05c1441e9f96870be1cc20b1162e3f5043b2e.1675734681.git.chenfeiyang@loongson.cn>
 <1e7da4e7-392d-4a9e-aa95-0599a0c84419@app.fastmail.com>
In-Reply-To: <1e7da4e7-392d-4a9e-aa95-0599a0c84419@app.fastmail.com>
From:   Feiyang Chen <chris.chenfeiyang@gmail.com>
Date:   Wed, 8 Feb 2023 10:09:48 +0800
Message-ID: <CACWXhK==yVZGZbY+3DUAuxL34=gKwQv20Cw3y1+QKir0D2F3EQ@mail.gmail.com>
Subject: Re: [PATCH 1/3] nolibc: Add statx() support to implement sys_stat()
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     w@1wt.eu, "Paul E. McKenney" <paulmck@kernel.org>,
        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
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 7 Feb 2023 at 22:31, Arnd Bergmann <arnd@arndb.de> wrote:
>
> 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.
>

Hi, Arnd,

I'd really like to make all architectures use sys_statx() instead
of sys_stat(). I just fear we might get dragged into a long discussion.
Can I send a patch series to do this later?

> > +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?
>

Yes, I will #include <linux/stat.h> here.

Thanks,
Feiyang

>     Arnd