Re: [RFC] FADV_REMOVE

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

On Sat, Aug 16, 2008 at 04:35:14PM +0100, David Woodhouse wrote:
> On Sat, 2008-08-16 at 16:33 +0100, David Woodhouse wrote:
> > Any particular reason why I shouldn't do this?
> 
> 
> ... and (something similar to) this...

I really don't like hooking this into fadvice.  facvice is to quote the
manpage to "predeclare an access pattern for file data".  Punching a
whole and discaring data doesn't really fall into that category for me.

> +	case LINUX_FADV_REMOVE:
> +		if (!(file->f_mode & FMODE_WRITE)) {
> +                        ret = -EBADF;
> +			goto out;
> +		}
> +		if ((offset & ~PAGE_MASK) || (len & ~PAGE_MASK)) {
> +			/* truncate_inode_pages_range() can't cope */
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		if (endbyte == -1) {
> +			/* FIXME: truncate to end */
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		if (!mapping->host || !mapping->host->i_op ||
> +		    !mapping->host->i_op->truncate_range) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		vmtruncate_range(mapping->host, offset, endbyte);

Calling vmtruncate_range directrly is a bad idea, we already carefull
don't do this in the truncate path (we do in the buffer write error
handling for some tme and it _does_ cause problems).  I'd rather have
a higher level punch hole functionality, and that litle bit of
functionaily in vmtruncate_range can be opencoded in the fs, especially
as the locking will be different for most non-trivial ones.

I know most of these problems already are there for MADV_REMOVE which
unfortunately went in without me notices, and is a rather tmpfs-specific
hack according to the commit message.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html