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

[Sage Weil]

On Tue, 15 Mar 2011, Andreas Dilger wrote:
> Should there be a "wait" argument or flag that allows an app to start 
> the syncfs(), do something, and then call again to wait for completion?
> 
> A more advanced implementation would have the "wait=0" call return an 
> opaque wait handle (e.g. transaction ID) and then calling back with this 
> handle as an argument would only wait if that handle hadn't committed 
> yet. That allows userspace to do efficient batching without having to 
> wait for the full sync to complete.

FWIW this is what the btrfs {START,WAIT}_SYNC ioctls do.  Because it ties 
into btrfs' transactions, there's an additional semantic that operations 
after the START_SYNC are not included in the committed state (this time 
around, at least).  Absent that (which I'm not sure maps cleanly onto many 
other file systems currently, and is of limited value when not used in 
concert with btrfs snapshots), I'm not sure a wait flag is helpful.  Is it 
any different than an application doing the syncfs() in a separate thread?

sage


> 
> Cheers, Andreas
> 
> On 2011-03-15, at 4:11, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> 
> > On Sun, Mar 13, 2011 at 09:29:17PM -0700, Sage Weil wrote:
> >> 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.
> > 
> > Perhaps we should consider propagating errors out to the user
> > application rather than discarding them in kernel and pretending we
> > can't ever have a write error?
> > 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@xxxxxxxxxxxxx
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
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/