Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753513AbcKBNVT (ORCPT <rfc822;w@1wt.eu>);
        Wed, 2 Nov 2016 09:21:19 -0400
Received: from imap.thunk.org ([74.207.234.97]:41654 "EHLO imap.thunk.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751861AbcKBNVR (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 2 Nov 2016 09:21:17 -0400
Date: Wed, 2 Nov 2016 09:21:14 -0400
From: "Theodore Ts'o" <tytso@mit.edu>
To: Hugh Dickins <hughd@google.com>
Cc: Dave Chinner <david@fromorbit.com>,
        Jakob Unterwurzacher <jakobunt@gmail.com>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: tmpfs returns incorrect data on concurrent pread() and truncate()
Message-ID: <20161102132114.amvwgjxq7x2b7ns5@thunk.org>
Mail-Followup-To: Theodore Ts'o <tytso@mit.edu>,
        Hugh Dickins <hughd@google.com>, Dave Chinner <david@fromorbit.com>,
        Jakob Unterwurzacher <jakobunt@gmail.com>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
References: <18e9fa0f-ec31-9107-459c-ae1694503f87@gmail.com>
 <alpine.LSU.2.11.1611011602340.1362@eggly.anvils>
 <20161102010147.GC9920@dastard>
 <alpine.LSU.2.11.1611011833130.2382@eggly.anvils>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.LSU.2.11.1611011833130.2382@eggly.anvils>
User-Agent: NeoMutt/20161014 (1.7.1)
X-SA-Exim-Connect-IP: <locally generated>
X-SA-Exim-Mail-From: tytso@thunk.org
X-SA-Exim-Scanned: No (on imap.thunk.org); SAEximRunCond expanded to false
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1887
Lines: 38

On Tue, Nov 01, 2016 at 06:38:26PM -0700, Hugh Dickins wrote:
> > Put simple: page locks are insufficient as a generic mechanism for
> > serialising filesystem operations. The locking required for this is
> > generally deeply filesystem implementation specific, so it's fine
> > that the VFS doesn't attempt to provide anything stricter than it
> > currently does....
> 
> I think you are saying that: xfs already provides the extra locking
> that avoids this issue; most other filesystems do not; but more can
> be expected to add that extra locking in the coming months?

The test program was using buffered reads, and ext4 is using
generic_file_read_iter(), so presumably adding file system specific
locking would require that file systems which use
generic_file_read_iter() would need to front-end it with the
additional required file system specific locks.  This is what XFS is
currently doing, but a quick git grep seems to show that besides XFS,
nfs, cifs, ceph and ocfs2 (i.e., XFS and remote file systems),
everyone else is using generic_file_read_iter() is using it directly
as read_iter method.

So if we think we really care about getting this case right, and if we
think the locking overhead in the uncontended case is sufficiently low
that it's worth fixing (it'll probably mean the bonnie benchmark will
take a hit, but that's a pretty crappy benchmark anyway :-), perhaps
we should be thinking about some way of adding a generic locking
mechanism so that most file systems will have a fighting chance of
getting this right.

Otherwise, perhaps the major file systems will add the appropriate
locking to the buffered I/O case (much more attention has been paid to
the direct I/O case, which is where more workloads have historically
cared) but I suspect as a percentage of the file systems in the Linux
kernel, they won't be getting this case right.

Cheers,

						- Ted