Re: [PATCH v3] introduce sys_syncfs to sync a single file system

[Indan Zupancic]

On Sat, March 12, 2011 18:32, Greg KH wrote:
> On Fri, Mar 11, 2011 at 08:10:01PM -0600, Jonathan Nieder wrote:
>> Indan Zupancic wrote:
>>
>> > I'm not pushing for any official convention, just what seems good taste.
>>
>> In cases like this, conventions (consistency and best practices) are
>> very important.
>>
>> > Less code added, less bloat. Architecture independent, no need to update
>> > all system call tables everywhere (all archs, libc versions and strace).
>> > Two files changed, instead of 7 (which only hooks up x86).
>>
>> Thanks for explaining.  Those do seem like good reasons to use a ioctl
>> instead of a new syscall.
>
> No, make it a syscall, it's more obvious and will be documented much
> better.

There is no such guarantee. Everyone seems to want to add this new syncfs,
but it's not even defined what it does. "Same as sync, but only on one fs"
is IMHO not good enough, because sync's behaviour is pretty badly documented,
and that's a system call. The sync_file_range argument effects are quite
well defined, on the other hand, unlike sync behaviour. You're right for
ioctls though.

If it has to be added, I'm in favour of adding it as a new sync_file_range
flag option, but wouldn't mind if it was added as some obscure ioctl either.

Do you think that widespread sync() usage is something good? Don't you
think sync(2) should be used as little as possible? And if so, why is
it any different than syncfs?

IMHO we're talking about some system call that shouldn't be used often, and
only rarely in very specific cases. They could use sync(2) with exactly the
same effect, but for performance reasons syncfs() is preferred. So instead
of extremely slow it will be very slow in some cases. Hurray, progress!

Next time you want an async version, and have to add a new systemcall with
a flag argument. So instead of going down that path, please add one with
a flag argument immediately. E.g:

syncfs(const char* path, int fd, int flags);

A NULL path, -1 fd and 0 flags is equal to sync (or something), and then
vary the options. But as it's for a file system, wouldn't it make more
sense to have just a path argument instead of an fd one?

Greetings,

Indan


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/