Re: [PATCH v8 2/3] cachestat: implement cachestat syscall

On Fri, Jan 27, 2023 at 04:46:38PM +0100, Arnd Bergmann wrote:

On Thu, Jan 26, 2023, at 18:53, Nhat Pham wrote:

quoted

SYNOPSIS
    #include <sys/mman.h>

    struct cachestat {
        __u64 nr_cache;
        __u64 nr_dirty;
        __u64 nr_writeback;
        __u64 nr_evicted;
        __u64 nr_recently_evicted;
    };

    int cachestat(unsigned int fd, off_t off, size_t len,
          unsigned int cstat_version, struct cachestat *cstat,
          unsigned int flags);

Is this "off_t off" argument intentionally limited to the old
32-bit type on 32-bit architectures? Unfortunately I fear 
there are no good options to pass an offset here:

- if you make it a 32-bit type, this breaks calling it from
  normal userspace that defines off_t as a 64-bit type

- if you change it to a 64-bit loff_t, there are three
  separate calling conventions for 64-bit, 32-bit with
  aligned register pairs and other 32-bit, plus you
  exceed the usual limit of six system call arguments

That's a good point, thanks for raising it, Arnd.

A separate problem may be the cstat_version argument, usually
we don't use interface versions but instead use a new
system call number if something changes in an incompatible
way.

I suppose from a userspace POV, a version argument vs calling a
separate syscall doesn't make much of a difference. So going with
loff_t and dropping cstat_version seems like a sensible way forward.

As an added bonus, versioning the syscall itself means the signature
can change in v2. This allows dropping the unused flags arg for now.

That would leave it at:

  int cachestat(unsigned int, loff_t, size_t len, struct cachestat *cstat);

and should we ever require extensions - new fields, flags - they would
come through a new cachestat2().

Would anybody have objections to that?