From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anton Altaparmakov Subject: Re: Access content of file via inodes Date: Wed, 06 Apr 2005 12:33:45 +0100 Message-ID: <1112787226.21605.27.camel@imp.csi.cam.ac.uk> References: Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: "Kathy KN (HK)" , Bryan Henderson , linux-fsdevel@vger.kernel.org Return-path: Received: from ppsw-2.csi.cam.ac.uk ([131.111.8.132]:57279 "EHLO ppsw-2.csi.cam.ac.uk") by vger.kernel.org with ESMTP id S262171AbVDFLeO (ORCPT ); Wed, 6 Apr 2005 07:34:14 -0400 To: Jeff Mahoney In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Jeff Mahoney wrote: > Kathy KN (HK) wrote: > > What I meant by via blocks is to gain knowledge of the physical > > blocks used by the inodes and retrieve the content from it directly, > > by accessing b_data. > > The problem with that approach is that some filesystems may store part > of the file outside of a complete block. For example, reiserfs "tails" > will respond with -ENOENT on ->bmap. For files smaller than 16k, they > are quite common. This is one not true and two wrong! Looking at reiserfs code in the current 2.6 kernel it does: .bmap = reiserfs_aop_bmap, Which is: static sector_t reiserfs_aop_bmap(struct address_space *as, sector_t block) { return generic_block_bmap(as, block, reiserfs_bmap) ; } And generic_block_bmap is: sector_t generic_block_bmap(struct address_space *mapping, sector_t block, get_block_t *get_block) { struct buffer_head tmp; struct inode *inode = mapping->host; tmp.b_state = 0; tmp.b_blocknr = 0; get_block(inode, block, &tmp, 0); return tmp.b_blocknr; } It ignores any errors from get_block() and always returns tmp.b_blocknr. Thus is get_block() fails, tmp.b_blocknr is 0 and hence 0 is returned, i.e. a sparse block. Which is complete rubbish... And get_block in this case in reiserfs is: static int reiserfs_bmap (struct inode * inode, sector_t block, struct buffer_head * bh_result, int create) { if (!file_capable (inode, block)) return -EFBIG; reiserfs_write_lock(inode->i_sb); /* do not read the direct item */ _get_block_create_0 (inode, block, bh_result, 0) ; reiserfs_write_unlock(inode->i_sb); return 0; } This will result in sparse blocks being returned whenever an error occurs. Not what is desired... The problem with ->bmap is that it cannot return error at all. It either returns 0 for sparse or >0 for real block. ->bmap is the most stupid interface I have ever seen... )-: If you ask me it should be removed from the kernel without notice. Let all applications that use it break. Who cares... It can always be replaced with a sensible interface that returns errors like -ESPARSE, -ENOTAPPLICABLE, -EIO, -ENOMEM, etc and doesn't assume that 0 is sparse... Best regards, Anton -- Anton Altaparmakov (replace at with @) Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/