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

["Arnd Bergmann" <arnd@arndb.de>]

On Fri, Jan 27, 2023, at 20:46, Johannes Weiner wrote:
> On Fri, Jan 27, 2023 at 04:46:38PM +0100, Arnd Bergmann wrote:
>> 
>> - 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);

There is still a problem of incompatible calling conventions:
on architectures that require even/odd register pairs, this would
end up like

int cachestat(unsigned int, long unused, u32 off_low, u32 off_high,
              size_t len, struct cachestat *cstat);

A more portable way to do this would be to pass the offset by
reference, but that makes it a bit awkward in userspace.

Or the arguments could be rearranged to put the low/high argument
pair first/second, third/fourth or fifth/sixth argument, at least
on the kernel ABI to avoid having another situation like
sys_arm_fadvise64_64.

> and should we ever require extensions - new fields, flags - they would
> come through a new cachestat2().
>
> Would anybody have objections to that?

If there is room for another argument, I would keep the 'flags'
as a way for extending in a compatible way, that is pretty standard
now, just not flags plus version.

      Arnd