From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH v3] vfs: fix a bug when we do some dio reads with append dio writes Date: Mon, 16 Dec 2013 11:01:55 +0100 Message-ID: <20131216100155.GD2446@quack.suse.cz> References: <1386413726-4610-1-git-send-email-wenqing.lz@taobao.com> <1387186654.2736.6.camel@menhir> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Zheng Liu , linux-fsdevel@vger.kernel.org, Jan Kara , Christoph Hellwig , Dmitry Monakhov , Dave Chinner , Alexander Viro , Zheng Liu To: Steven Whitehouse Return-path: Received: from cantor2.suse.de ([195.135.220.15]:37588 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750741Ab3LPUBu (ORCPT ); Mon, 16 Dec 2013 15:01:50 -0500 Content-Disposition: inline In-Reply-To: <1387186654.2736.6.camel@menhir> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon 16-12-13 09:37:34, Steven Whitehouse wrote: > Hi, > > On Sat, 2013-12-07 at 18:55 +0800, Zheng Liu wrote: > > From: Zheng Liu > > > > Currently we check i_size in buffered read path after we know the page > > is update. But it could return some zero-filled pages to the userspace > > when we do some append dio writes. We could use the following code > > snippet to reproduce this problem in a ext2/3/4 filesystem. > > > If the page is not uptodate, then neither (potentially) is i_size, since > the underlying fs has not had a chance to get its own locks and update > the inode size. Hum, this isn't really about page being uptodate or not, is it? It is more about the fact that gfs2 updates i_size only from gfs2_readpage() function when obtaining inode lock. Or do you know about other filesystem having the same problem as gfs2? E.g. ocfs2 updates i_size from ocfs2_file_aio_read() before calling generic_file_aio_read(), cannot gfs2 do something similar? > I suspect that the correct fix would be to implement ->launder_page to > avoid the race that you've identified here, if I've understood it > correctly, I don't understand how that would work. Can you elaborate a bit? Here the problem is i_size gets extended by DIO write during buffered read (which is a fallback from DIO read) so we return zero-filled page instead of either data filled page or short read. I don't see where launder_page() would come into the picture... Honza -- Jan Kara SUSE Labs, CR