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

On Mon, 14 Mar 2011, Indan Zupancic wrote:

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.

How about the man page below?  I tried to avoid the somewhat antiquated 
implementation specific terminology in the sync(2) man page.

I think adding this functionality into sync_file_range(2) is forcing 
unrelated functionality into an existing interface; sync_file_range 
operates on _files_, not an entire file system.  With each API addition it 
is more important to make the interface simple and intuitive than to 
minimize the size of our patches.  IMO that's why a new syscall is 
preferable to, say, an equivalent ioctl.

Thanks-
sage


.TH SYNCFS 2 2011-03-13 "Linux" "Linux Programmer's Manual"
.SH NAME
syncfs \- commit cached file system state to stable storage
.SH SYNOPSIS
.B #include <unistd.h>
.sp
.B void syncfs(int fd);
.SH DESCRIPTION
.BR syncfs ()
flushes any cached data modifications to the file system containing the 
file referenced by the file descriptor
.I fd
to stable storage (usually a disk).  This includes the results of any
file modifications or other file system operations that have completed
prior to the call to
.BR syncfs(2).
This is similar to 
.BR sync(2),
but will commit changes for only a single file system instead of all
mounted file systems.
.SH ERRORS
This function is always successful.
.SH "SEE ALSO"
.BR bdflush (2),
.BR fdatasync (2),

4 more lines

On Mon, March 14, 2011 05:29, Sage Weil wrote:

On Mon, 14 Mar 2011, Indan Zupancic wrote:

quoted

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.

How about the man page below?  I tried to avoid the somewhat antiquated
implementation specific terminology in the sync(2) man page.

Good enough.

I think adding this functionality into sync_file_range(2) is forcing
unrelated functionality into an existing interface; sync_file_range
operates on _files_, not an entire file system.  With each API addition
it is more important to make the interface simple and intuitive than
to minimize the size of our patches.  IMO that's why a new syscall
is preferable to, say, an equivalent ioctl.

Your new syncfs also operates on files, it's only the name of sync_file_range
which is not very fitting. Other than that, syncing files in weird, nonstandard
ways is what it does and in that sense it's a good place for syncfs functionality.

But hey, it seems I'm the only one favouring that approach, and as you don't
mind the size of your patch, make sure to add support for syncfs to all other
23 archs too in your next patch submission.

Take care,

Indan

Thanks-
sage


.TH SYNCFS 2 2011-03-13 "Linux" "Linux Programmer's Manual"
.SH NAME
syncfs \- commit cached file system state to stable storage
.SH SYNOPSIS
.B #include <unistd.h>
.sp
.B void syncfs(int fd);
.SH DESCRIPTION
.BR syncfs ()
flushes any cached data modifications to the file system containing the
file referenced by the file descriptor
.I fd
to stable storage (usually a disk).  This includes the results of any
file modifications or other file system operations that have completed
prior to the call to
.BR syncfs(2).
This is similar to
.BR sync(2),
but will commit changes for only a single file system instead of all
mounted file systems.
.SH ERRORS
This function is always successful.
.SH "SEE ALSO"
.BR bdflush (2),
.BR fdatasync (2),
.BR fsync (2),
.BR sync (2),
.BR sync (8),
.BR update (8)

On Mar 14, 2011, at 5:27 AM, Indan Zupancic wrote:

Your new syncfs also operates on files, it's only the name of sync_file_range
which is not very fitting. Other than that, syncing files in weird, nonstandard
ways is what it does and in that sense it's a good place for syncfs functionality.

It operates on a file system.  It takes a file descriptor as an argument.  (Much
like the statfs(2) system call.)  There's a difference. 

- Ted