lkml.org 
[lkml]   [2023]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v8 2/3] cachestat: implement cachestat syscall
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

\
 
 \ /
  Last update: 2023-03-27 00:00    [W:0.040 / U:0.612 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site