linux-ext4 - Re: [PATCH 01/15 v2] mm: add invalidatepage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20120904164316.6e058cbe.akpm@linux-foundation.org>
Date:	Tue, 4 Sep 2012 16:43:16 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
	tytso@....edu, hughd@...gle.com, linux-mm@...ck.org
Subject: Re: [PATCH 01/15 v2] mm: add invalidatepage_range address space
 operation

On Fri, 31 Aug 2012 18:21:37 -0400
Lukas Czerner <lczerner@...hat.com> wrote:

> Currently there is no way to truncate partial page where the end
> truncate point is not at the end of the page. This is because it was not
> needed and the functionality was enough for file system truncate
> operation to work properly. However more file systems now support punch
> hole feature and it can benefit from mm supporting truncating page just
> up to the certain point.
> 
> Specifically, with this functionality truncate_inode_pages_range() can
> be changed so it supports truncating partial page at the end of the
> range (currently it will BUG_ON() if 'end' is not at the end of the
> page).
> 
> This commit add new address space operation invalidatepage_range which
> allows specifying length of bytes to invalidate, rather than assuming
> truncate to the end of the page. It also introduce
> block_invalidatepage_range() and do_invalidatepage)range() functions for
> exactly the same reason.
> 
> The caller does not have to implement both aops (invalidatepage and
> invalidatepage_range) and the latter is preferred. The old method will be
> used only if invalidatepage_range is not implemented by the caller.
> 
> ...
>
> +/**
> + * do_invalidatepage_range - invalidate range of the page
> + *
> + * @page: the page which is affected
> + * @offset: start of the range to invalidate
> + * @length: length of the range to invalidate
> +  */
> +void do_invalidatepage_range(struct page *page, unsigned int offset,
> +			     unsigned int length)
> +{
> +	void (*invalidatepage_range)(struct page *, unsigned int,
> +				     unsigned int);
>  	void (*invalidatepage)(struct page *, unsigned long);
> +
> +	/*
> +	 * Try invalidatepage_range first
> +	 */
> +	invalidatepage_range = page->mapping->a_ops->invalidatepage_range;
> +	if (invalidatepage_range) {
> +		(*invalidatepage_range)(page, offset, length);
> +		return;
> +	}
> +
> +	/*
> +	 * When only invalidatepage is registered length + offset must be
> +	 * PAGE_CACHE_SIZE
> +	 */
>  	invalidatepage = page->mapping->a_ops->invalidatepage;
> +	if (invalidatepage) {
> +		BUG_ON(length + offset != PAGE_CACHE_SIZE);
> +		(*invalidatepage)(page, offset);
> +	}
>  #ifdef CONFIG_BLOCK
> -	if (!invalidatepage)
> -		invalidatepage = block_invalidatepage;
> +	if (!invalidatepage_range && !invalidatepage)
> +		block_invalidatepage_range(page, offset, length);
>  #endif
> -	if (invalidatepage)
> -		(*invalidatepage)(page, offset);
>  }

This interface is ...  strange.  If the caller requests a
non-page-aligned invalidateion against an fs which doesn't implement
->invalidatepage_range then the kernel goes BUG.  So the caller must
know beforehand that the underlying fs _does_ implement
->invalidatepage_range.

For practical purposes, this implies that invalidation of a
non-page-aligned region will only be performed by fs code, because the
fs implicitly knows that it implements ->invalidatepage_range.

However this function isn't exported to modules, so scratch that.

So how is calling code supposed to determine whether it can actually
_use_ this interface?



Also...  one would obviously like to see the old ->invalidatepage() get
removed entirely.  But about 20 filesystems implement
->invalidatepage() and implementation of ->invalidatepage_range() is
non-trivial and actually unnecessary.

So I dunno.  Perhaps we should keep ->invalidatepage() and
->invalidatepage_range() completely separate.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html