Nick Piggin <npiggin@suse.de> wrote:

quoted text
> -	ASSERTCMP(start + len, <=, PAGE_SIZE);
> +	ASSERTCMP(len, <=, PAGE_CACHE_SIZE);

Do you guarantee this will work if PAGE_CACHE_SIZE != PAGE_SIZE?  If not, you
can't make this particular change.

Do we ever intend to have PAGE_CACHE_SIZE != PAGE_SIZE?  If not, then surely
the former is redundant and should scrapped to avoid confusion?

quoted text
> +	i_size = i_size_read(&vnode->vfs_inode);
> +	if (pos + len > i_size)
> +		eof = i_size;
> +	else
> +		eof = PAGE_CACHE_SIZE;
> +
> +	ret = afs_vnode_fetch_data(vnode, key, 0, eof, page);

That can't be right, surely.  Either 'eof' is the size of the file or it's the
length of the data to be read.  It can't be both.  The first case needs eof
masking off.  Also, 'eof' isn't a good choice of name.  'len' would be better
were it not already taken:-/

I notice you removed the stuff that clears holes in the page to be written.  Is
this is now done by the caller?

I notice also that you use afs_fill_page() in place of afs_prepare_page() to
prepare a page.  You can't do this if the region to be filled currently lies
beyond the server's idea of EOF.

I'll try and get a look at fixing this patch tomorrow.

David
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html