Messages in this thread | | | Date | Tue, 28 Jun 2011 18:56:41 -0700 | Subject | Re: [PATCH] writeback: Don't wait for completion in writeback_inodes_sb_nr | From | Curt Wohlgemuth <> |
| |
Hi Dave:
Thanks for the response.
On Tue, Jun 28, 2011 at 5:54 PM, Dave Chinner <david@fromorbit.com> 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@google.com> > > 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@fromorbit.com > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |