From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andreas Dilger <adilger@sun.com>
Subject: Re: [PATCH 5/5] generic block based fiemap implementation
Date: Sun, 25 May 2008 02:00:51 -0600
Message-ID: <20080525080051.GK3516@webber.adilger.int>
References: <20080525000235.GO8325@wotan.suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7BIT
Cc: linux-fsdevel@vger.kernel.org, Josef Bacik <jbacik@redhat.com>
To: Mark Fasheh <mfasheh@suse.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from sca-es-mail-1.Sun.COM ([192.18.43.132]:41518 "EHLO
	sca-es-mail-1.sun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752666AbYEYIAx (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Sun, 25 May 2008 04:00:53 -0400
Received: from fe-sfbay-10.sun.com ([192.18.43.129])
	by sca-es-mail-1.sun.com (8.13.7+Sun/8.12.9) with ESMTP id m4P80rfk008970
	for <linux-fsdevel@vger.kernel.org>; Sun, 25 May 2008 01:00:53 -0700 (PDT)
Received: from conversion-daemon.fe-sfbay-10.sun.com by fe-sfbay-10.sun.com
 (Sun Java System Messaging Server 6.2-8.04 (built Feb 28 2007))
 id <0K1E00801ZJDOQ00@fe-sfbay-10.sun.com> (original mail from adilger@sun.com)
 for linux-fsdevel@vger.kernel.org; Sun, 25 May 2008 01:00:53 -0700 (PDT)
In-reply-to: <20080525000235.GO8325@wotan.suse.de>
Content-disposition: inline
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On May 24, 2008  17:02 -0700, Mark Fasheh wrote:
> +int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> +		u64 start, u64 len)
> +{
> +	return generic_block_fiemap(inode, fieinfo, start, len,
> +				    ext3_get_block);
> +}

We can wire up ext2 in the same way, and likely most of the other filesystems.


#define GENERIC_FIEMAP_FLAG_SUPP (FIEMAP_FLAG_NUM_EXTENTS | FIEMAP_FLAG_SYNC |
				  FIEMAP_FLAG_HSM_READ | FIEMAP_FLAG_LUN_ORDER)

> +int generic_block_fiemap(struct inode *inode,
> +			 struct fiemap_extent_info *fieinfo, u64 start,
> +			 u64 len, get_block_t *get_block)
> +{
> +	int just_count = fieinfo->fi_flags & FIEMAP_FLAG_NUM_EXTENTS;

Don't need this anymore.


	if (fieinfo->fi_flags & ~GENERIC_FIEMAP_FLAG_SUPP) {
		fieinfo->fi_flags &= ~GENERIC_FIEMAP_FLAG_SUPP;
		return -EBADF;
	}
		
> +		/* HOLE */
> +		if (!tmp.b_blocknr) {
> +			if (length <= 0) {
> +				if (just_count) {
> +					fieinfo->fi_extents_mapped++;
> +					break;
> +				}
> +				flags = FIEMAP_EXTENT_LAST;
> +				ret = fiemap_fill_next_extent(fieinfo, logical,
> +							      phys, size,
> +							      flags, 0);
> +				break;

Can remove the just_count check, fiemap_fill_next_extent() will handle it.

> +		} else {
> +			if (length <= 0 && size && !just_count) {
> +				ret = fiemap_fill_next_extent(fieinfo, logical,
> +							      phys, size,
> +							      flags, 0);
> +				if (ret)
> +					break;
> +			}
> +
> +			logical = blk_to_logical(inode, start_blk);
> +			phys = blk_to_logical(inode, tmp.b_blocknr);
> +			size = tmp.b_size;
> +			flags = 0;
> +
> +			length -= tmp.b_size;
> +			start_blk += logical_to_blk(inode, size);
> +
> +			if (just_count) {
> +				fieinfo->fi_extents_mapped++;
> +				if (fieinfo->fi_extents_mapped ==
> +				    fieinfo->fi_extents_max)
> +					break;
> +				continue;
> +			}

Can also remove this just_count check, which is incorrect in any case.
The fi_extents_max is only when storing extents in the array, it isn't
meaningful when only counting extents.

More important is the extent merging I proposed for fiemap_fill_next_extent()
for filesystems that only return single blocks, so we want to always
call it from the generic_block_fiemap() code.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.