From mboxrd@z Thu Jan 1 00:00:00 1970 From: Curt Wohlgemuth Subject: Re: [PATCH] writeback: Don't wait for completion in writeback_inodes_sb_nr Date: Tue, 28 Jun 2011 18:56:41 -0700 Message-ID: References: <1309304616-8657-1-git-send-email-curtw@google.com> <20110629005422.GQ32466@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Al Viro , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Dave Chinner Return-path: Received: from smtp-out.google.com ([74.125.121.67]:55912 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751390Ab1F2B4p convert rfc822-to-8bit (ORCPT ); Tue, 28 Jun 2011 21:56:45 -0400 Received: from hpaq3.eem.corp.google.com (hpaq3.eem.corp.google.com [172.25.149.3]) by smtp-out.google.com with ESMTP id p5T1uhWY025555 for ; Tue, 28 Jun 2011 18:56:43 -0700 Received: from qwf6 (qwf6.prod.google.com [10.241.194.70]) by hpaq3.eem.corp.google.com with ESMTP id p5T1uGKJ030849 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Tue, 28 Jun 2011 18:56:42 -0700 Received: by qwf6 with SMTP id 6so630554qwf.16 for ; Tue, 28 Jun 2011 18:56:42 -0700 (PDT) In-Reply-To: <20110629005422.GQ32466@dastard> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi Dave: Thanks for the response. On Tue, Jun 28, 2011 at 5:54 PM, Dave Chinner wro= te: > 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 >> >> =A0 =A0 =A0 =A0 wait_for_completion(&done); >> >> which should not be done, as this is not called for data >> integrity sync. >> >> Signed-off-by: Curt Wohlgemuth > > 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-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html