Skip to contentSkip to conversation

Possible bug in btrfs_file_aio_write()

DISCUSSION Akshat Aranya 1 participant 1 message Feb 2, 2011 Btrfs file system

lore
Share & tools
lore
#0 lore raw

Hi,

I'm looking at the code for btrfs_file_aio_write(). Specifically, the following lines:

	first_index = pos >> PAGE_CACHE_SHIFT;
	last_index = (pos + iov_iter_count(&i)) >> PAGE_CACHE_SHIFT;

	/*
	 * there are lots of better ways to do this, but this code
	 * makes sure the first and last page in the file range are
	 * up to date and ready for cow
	 */
	if ((pos & (PAGE_CACHE_SIZE - 1))) {
		pinned[0] = grab_cache_page(inode->i_mapping, first_index);
		if (!PageUptodate(pinned[0])) {
			ret = btrfs_readpage(NULL, pinned[0]);
			BUG_ON(ret);
			wait_on_page_locked(pinned[0]);
		} else {
			unlock_page(pinned[0]);
		}
	}
	if ((pos + iov_iter_count(&i)) & (PAGE_CACHE_SIZE - 1)) {
		pinned[1] = grab_cache_page(inode->i_mapping, last_index);
		if (!PageUptodate(pinned[1])) {
			ret = btrfs_readpage(NULL, pinned[1]);
			BUG_ON(ret);
			wait_on_page_locked(pinned[1]);
		} else {
			unlock_page(pinned[1]);
		}
	}

Am I missing something, or is there an off-by-one error over here? The last byte in the written region would be "pos + iov_iter_count(&i) - 1", not "pos + iov_iter_count(&i)". I verified this by writing 4096 bytes at offset 0. first_index evaluates to 0 and last_index evaluates to 1.

-Akshat