# Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - List: linux-api - 5 messages, 5 authors - Page: https://ratatoskr.run/linux-api/2011/03/51669/t ## Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - From: Andrew Morton \ - Date: 2011-03-14 20:10:42 - Message: https://ratatoskr.run/linux-api/2011/03/51669.md ```text On Mon, 14 Mar 2011 02:56:52 +0100 (CET) "Indan Zupancic" [ref] wrote: > 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. I think the semantics of sync are easily enough defined, even if they're not well-defined in the documentation: all data which was dirty at the time sync() was called will be written back and accessible when the sync() returns. I do agree that this should be a standalone syscall, not grafted into sync_file_range() or into an ioctl. That being said, we have two similar-looking-but-quite-different "sync" concepts in the kernel. One is "sync for data integrity" and the other is "sync to reduce the dirty memory load". The latter is not a data integrity thing - it is a resource management thing. There might one day be a requirement to be able to initiate a resource-management-style writeback against a whole filesystem. When that happens, we'll regret not having added a "mode" argument to sys_syncfs(). Or maybe not - given that we're syncing the entire fs and that sync_filesystem() does the two-pass "write for data cleaning then write for data integrity" thing, it could be that a syncfs-for-data-cleaning operation has little performance benefit over a syncfs-for-data-integrity operation. ``` ## Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - From: Artem Bityutskiy \ - Date: 2011-03-14 20:30:03 - Message: https://ratatoskr.run/linux-api/2011/03/51671.md ```text On Mon, 2011-03-14 at 13:10 -0700, Andrew Morton wrote: > There might one day be a requirement to be able to initiate a > resource-management-style writeback against a whole filesystem. When > that happens, we'll regret not having added a "mode" argument to > sys_syncfs(). I think Indan is right about an additional argument which could be used for future extensions, what is the problem adding it? -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ``` ## Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - From: Ted Ts'o \ - Date: 2011-03-14 21:11:19 - Message: https://ratatoskr.run/linux-api/2011/03/51675.md ```text On Mon, Mar 14, 2011 at 01:10:42PM -0700, Andrew Morton wrote: > > There might one day be a requirement to be able to initiate a > resource-management-style writeback against a whole filesystem. When > that happens, we'll regret not having added a "mode" argument to > sys_syncfs(). I'm a bit nervous about exposing WB_SYNC_NONE to userspace, because its semantics are *definitely* hard to describe. For example, at the moment if you do a WB_SYNC_NONE writeback, the writeback code will clamp the amount of data written back for each inode to MAX_WRITEBACK_PAGES (1024) pages. Do we want to document that? Probably not! But if we don't document it, what can userspace expect? If you just issue a writeback_inodes_sb(), it's not the case that it will start a process that will eventually write out everything (i.e., it's not the equivalent of a non-blocking data integrity sync). It just means, "write out some stuff". I could imagine userspace wanting to start a non-blocking writeout of all data blocking pages, and which doesn't cause queue flush / barrier requests. (i.e., a non-blocking-non-barrier-issuing-but-otherwise-a- data-integrity writeback) But that's not something that the current writeback machinery can do easily, at least not today. It wouldn't hurt to have a "flags" field which we could expand later --- but that can lead to portability headaches for userspace programs that don't know whether a particular kernel is going to support a particular flag or not. So it's certainly not a panacea. - Ted ``` ## Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - From: Andrew Morton \ - Date: 2011-03-14 21:21:41 - Message: https://ratatoskr.run/linux-api/2011/03/51676.md ```text On Mon, 14 Mar 2011 17:11:19 -0400 "Ted Ts'o" [ref] wrote: > On Mon, Mar 14, 2011 at 01:10:42PM -0700, Andrew Morton wrote: > > > > There might one day be a requirement to be able to initiate a > > resource-management-style writeback against a whole filesystem. When > > that happens, we'll regret not having added a "mode" argument to > > sys_syncfs(). > > I'm a bit nervous about exposing WB_SYNC_NONE to userspace, because > its semantics are *definitely* hard to describe. For example, at the > moment if you do a WB_SYNC_NONE writeback, the writeback code will > clamp the amount of data written back for each inode to > MAX_WRITEBACK_PAGES (1024) pages. Wha? It does? When did that get broken? > Do we want to document that? > Probably not! But if we don't document it, what can userspace expect? > > If you just issue a writeback_inodes_sb(), it's not the case that it > will start a process that will eventually write out everything (i.e., > it's not the equivalent of a non-blocking data integrity sync). It > just means, "write out some stuff". > > I could imagine userspace wanting to start a non-blocking writeout of > all data blocking pages, and which doesn't cause queue flush / barrier > requests. (i.e., a non-blocking-non-barrier-issuing-but-otherwise-a- > data-integrity writeback) But that's not something that the current > writeback machinery can do easily, at least not today. Well. Current implementation shortcomings don't carry a lot of weight when designing a permanent interface. > It wouldn't hurt to have a "flags" field which we could expand later > --- but that can lead to portability headaches for userspace programs > that don't know whether a particular kernel is going to support a > particular flag or not. So it's certainly not a panacea. I don't see a need to add an arg to syncfs() really. But we should demonstrate that we've thought about it ;) ``` ## Re: \[PATCH v3\] introduce sys\_syncfs to sync a single file system - From: Arnd Bergmann \ - Date: 2011-03-14 21:23:29 - Message: https://ratatoskr.run/linux-api/2011/03/51677.md ```text On Monday 14 March 2011 22:11:19 Ted Ts'o wrote: > It wouldn't hurt to have a "flags" field which we could expand later > --- but that can lead to portability headaches for userspace programs > that don't know whether a particular kernel is going to support a > particular flag or not. So it's certainly not a panacea. I think adding an unused flags argument can't hurt. We could be fancy and ignore half the bits but bail out on the other half with -EINVAL. That would make it possible to add both compatible (default being full sync on old kernels) and incompatible (getting rejected on old kernels) flags. Arnd ```