From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch 2/4] fs: fsync inode dirty race fix Date: Tue, 23 Nov 2010 00:43:15 +1100 Message-ID: References: <20101122130507.GC12716@amd> <20101122130637.GD12716@amd> <20101122132318.GB25321@infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Nick Piggin , linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from mail-wy0-f174.google.com ([74.125.82.174]:44177 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754768Ab0KVNnX convert rfc822-to-8bit (ORCPT ); Mon, 22 Nov 2010 08:43:23 -0500 Received: by wyb28 with SMTP id 28so7028046wyb.19 for ; Mon, 22 Nov 2010 05:43:21 -0800 (PST) In-Reply-To: <20101122132318.GB25321@infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Nov 23, 2010 at 12:23 AM, Christoph Hellwig = wrote: >> =A0 =A0 =A0 ret =3D sync_mapping_buffers(inode->i_mapping); >> - =A0 =A0 if (!(inode->i_state & I_DIRTY)) >> - =A0 =A0 =A0 =A0 =A0 =A0 return ret; >> - =A0 =A0 if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) >> - =A0 =A0 =A0 =A0 =A0 =A0 return ret; >> - >> >> =A0 =A0 =A0 err =3D sync_inode_metadata(inode, 1); > > This makes fdatasync equivalent to fsync, which means a huge drop > in performance for database an virtualization performance. > > I think the right aproach is to extend the sync_inode_metadata to > writeback_single_inode chain with a datasync parameter so that we > can do the correct decision there. I was leaning towards using the new inode writeback helpers, then filesystems should use them to check inode dirty state without races. You could still move datasync into the writeback function, but it wouldn't be a requirement. More complex filesystem schemes seem to need to actually test these flags. > Note that there are a lot more fsync implementations than just > generic_file_fsync and exofs_fsync that have the same issue. Yes, they require patch 3/4, and a lot more testing than I had time to do tonight. I'll send out some patches tomorrow. -- 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