From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Mason Subject: Re: [PATCH 4 of 7] Turn the DIO lock_type parameter into a flags field Date: Wed, 1 Nov 2006 20:02:25 -0500 Message-ID: <20061102010225.GG31240@think.oraclecorp.com> References: <20061101225858.GO8394166@melbourne.sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, akpm@osdl.org, zach.brown@oracle.com, Suparna Bhattacharya Return-path: Received: from agminet01.oracle.com ([141.146.126.228]:29792 "EHLO agminet01.oracle.com") by vger.kernel.org with ESMTP id S1752393AbWKBBCb (ORCPT ); Wed, 1 Nov 2006 20:02:31 -0500 To: David Chinner Content-Disposition: inline In-Reply-To: <20061101225858.GO8394166@melbourne.sgi.com> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Thu, Nov 02, 2006 at 09:58:58AM +1100, David Chinner wrote: [ ...] Thanks for the review, I'll incorporate these suggestions into the next patch set. > > + > > + /* > > + * the placeholder code does filemap_write_and_wait, so if we > > + * aren't using placeholders we have to do it here > > + */ > > + if (!(dio->flags & DIO_PLACEHOLDERS) && end > offset) { > > + struct address_space *mapping = iocb->ki_filp->f_mapping; > > retval = filemap_write_and_wait_range(mapping, offset, end - 1); > > if (retval) > > goto out; > > So that means XFS will now do three cache flushes on write (one in > xfs_write(), one in generic_file_direct_IO() and yet another here.... I removed the flush before starting the IO in generic_file_direct_IO because that is now done by the placeholders. When placeholders aren't used, we need the flush to match the old behavior. I added the flush in generic_file_direct_IO after calling the direct_IO func because invalidate_page_range2 requires the data to be flushed first. Otherwise it fails to free a dirty page with dirty buffers and spits out -EIO. This only happens when mapping->nrpages > 0, so it should be very low cost in the common case. It's messy, I'll try to refactor things a bit to make it cleaner (same goes for i_mutex drop/lock). -chris