From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andreas Dilger <adilger@sun.com>
Subject: Re: [RFC][PATCH] Implement SEEK_HOLE/SEEK_DATA
Date: Wed, 28 Nov 2007 16:49:17 -0700
Message-ID: <20071128234917.GG4913@webber.adilger.int>
References: <20071128200206.GB3977@dhcp243-37.rdu.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-fsdevel@vger.kernel.org
To: Josef Bacik <jbacik@redhat.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail.clusterfs.com ([74.0.229.162]:53495 "EHLO
	mail.clusterfs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1761064AbXK1XtT (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Wed, 28 Nov 2007 18:49:19 -0500
Content-Disposition: inline
In-Reply-To: <20071128200206.GB3977@dhcp243-37.rdu.redhat.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

On Nov 28, 2007  15:02 -0500, Josef Bacik wrote:
> +static loff_t generic_seek_hole_data(struct file *file, loff_t offset,
> +				     int origin)
> +{
> +	loff_t retval = -ENXIO;
> +	struct inode *inode = file->f_mapping->host;
> +	sector_t block, found_block;
> +	sector_t last_block = (i_size_read(inode) - 1) >> inode->i_blkbits;
> +	int want = (origin == SEEK_HOLE) ? 0 : 1;
> +
> +	for (block = offset >> inode->i_blkbits; block <= last_block; block++) {
> +		found_block = bmap(inode, block);
> +
> +		if (!!found_block == want) {
> +			retval = block << inode->i_blkbits;
> +			break;
> +		}
> +	}

The problem with this is that it doesn't know anything about the filesystem,
and might spin for ages in a tight loop for a large sparse or dense file.

Almost any fs would have to implement ->seek_hole_data() for this to be
useful, at which point we might as well just report -EINVAL to the
application or glibc (since this needs to be handled anyways).

Also, what about network filesystems that don't support bmap?  They will
show up as entirely holes, since bmap() returns 0 if ->bmap is not
supported.  That could lead to serious data loss, if e.g. a filesystem
is being backed up with a SEEK_DATA-aware tool.

In light of this, it makes much more sense to only support this
functionality if the filesystem explicitly adds it, instead of pretending
that it works when there are sharp pointy things in the dark corners.

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