Hi Dave: Thanks for the response. On Tue, Jun 28, 2011 at 5:54 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > On Tue, Jun 28, 2011 at 04:43:35PM -0700, Curt Wohlgemuth wrote: >> Contrary to the comment block atop writeback_inodes_sb_nr(), >> we *were* calling >> >> wait_for_completion(&done); >> >> which should not be done, as this is not called for data >> integrity sync. >> >> Signed-off-by: Curt Wohlgemuth <curtw@xxxxxxxxxx> > > The comment says it does not wait for IO to be -completed-. > > The function as implemented waits for IO to be *submitted*. > > This provides the callers with same blocking semantics (i.e. request > queue full) as if the caller submitted the IO themselves. The code > that uses this function rely on this blocking to delay the next set > of operations they do until after IO has been started, so removing > the completion will change their behaviour significantly. I don't quite understand this. It's true that all IO done as a result of calling wb_writeback() on this work item won't finish before the completion takes place, but sending all those pages in flight *will* take place. And that's a lot of time. To wait on this before we then call sync_inodes_sb(), and do it all over again, seems odd at best. Pre-2.6.35 kernels would start non-integrity sync writeback and immediately return, which would seem like a reasonable "prefetch-y" thing to do, considering it's going to be immediately followed by a data integrity sync writeback operation. The post 2.6.35 semantics are fine; but then I don't understand why we do both a __sync_filesystem(0) followed by a __sync_filesystem(1) (in the case of sync(2)). It doesn't seem to be any safer or more correct to me; why not just do the data integrity sync writeback and call it a day? Thanks, Curt > > 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