From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH 1/1] fiemap: move i_op->fiemap() locking to the ioctl_fiemap() Date: Sat, 22 Sep 2012 08:59:10 +1000 Message-ID: <20120921225910.GB20960@dastard> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: sandeen@redhat.com, viro@zeniv.linux.org.uk, swhiteho@redhat.com, tytso@mit.edu, linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, zohar@linux.vnet.ibm.com To: Dmitry Kasatkin Return-path: Received: from ipmail05.adl6.internode.on.net ([150.101.137.143]:30259 "EHLO ipmail05.adl6.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754581Ab2IUW7N (ORCPT ); Fri, 21 Sep 2012 18:59:13 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Sep 21, 2012 at 05:14:28PM +0300, Dmitry Kasatkin wrote: > On some filesystems calling i_op->fiemap() takes the i_mutex, > on others it doesn't. Exactly by design. Many fiesystems don't require the i_mutex for fiemap to be safe. e.g. the extent map is not protected by the i_mutex in XFS or ext4, and so holding the i_mutex over fiemap is incorrect. As a result, extent maps can change even when someone is holding the i_mutex. Any design that requires i_mutex to provide stable, unchanging extent maps is fundamentally broken.... So that's a NACK from me... > This change is needed in preparation for EVM to include a hash of > the extent data to be used in the HMAC calculation. EVM is called > when the i_mutex has already been taken. What's EVM? What's HMAC? and what is it actually checksumming? The extent map, or the data in the file? And if it is the extent map, what's the purpose of using the extent map for this? You need to explain how this is intended to be used, not use TLAs that people unfamiliar with your application will not understand... Cheers, Dave. -- Dave Chinner david@fromorbit.com