From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeffrey Layton Subject: Re: [PATCH 2/2] locks: make locks_mandatory_area check for file-private locks Date: Mon, 10 Mar 2014 13:31:18 -0400 Message-ID: <20140310133118.4f41dea8@ipyr.poochiereds.net> References: <1394458607-23579-1-git-send-email-jlayton@redhat.com> <1394458607-23579-3-git-send-email-jlayton@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Linux FS Devel , Bruce Fields , trond.myklebust@primarydata.com To: Andy Lutomirski Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47638 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753584AbaCJRb0 (ORCPT ); Mon, 10 Mar 2014 13:31:26 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, 10 Mar 2014 10:07:10 -0700 Andy Lutomirski wrote: > On Mon, Mar 10, 2014 at 6:36 AM, Jeff Layton > wrote: > > Allow locks_mandatory_area() to handle file-private locks correctly. > > If there is a file-private lock set on an open file and we're doing > > I/O via the same, then that should not cause anything to block. > > > > Handle this by first doing a non-blocking FL_ACCESS check for a > > file-private lock, and then fall back to checking for a classic > > POSIX lock (and possibly blocking). > > This is ugly, but it doesn't seem much worse than the existing > code. :) > > --Andy My sentiments exactly. The alternative would to add some mechanism to check for locks that are owned by more than one owner without dropping the i_lock in between. That's doable but it'd be pretty ugly too, and you'd still have the existing races to contend with. FWIW, now that I've looked at this code it seems possible to make mandatory locking more or less race-free by having reads and writes both implicitly set a special sort of lock on the file: - a read lock (for both the read and write cases) - one that wouldn't conflict with existing locks held by same process or with file-private locks set on the same fd - but...the same lock could not be merged with either of the above (and would therefore need a different sort of ownership model). Have rw_verify_area set such a blocking lock on the file and then remove it once the read or write was done. It doesn't sound too hard to do, but performance would likely suck. That said, I don't feel terribly motivated to fix mandatory locking aside from doing my best to avoid the problem that Trond pointed out. -- Jeff Layton