* Re: mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)
@ 2006-12-21 13:27 Martin Johansson
0 siblings, 0 replies; only message in thread
From: Martin Johansson @ 2006-12-21 13:27 UTC (permalink / raw)
To: linux-kernel
> On Wed, 20 Dec 2006, Linus Torvalds wrote:
> Martin, Andrei, does this make any difference for your corruption cases?
Hi!
I've been watching this issue since I'm experiencing rtorrent corruption
since 2.6.19.
Details: i386, UP, no preempt:
kungen:/proc# zgrep PREEMPT config.gz
CONFIG_PREEMPT_NONE=y
# CONFIG_PREEMPT_VOLUNTARY is not set
# CONFIG_PREEMPT is not set
kungen:/proc# uname -a
Linux kungen.fatbob.nu 2.6.19.1 #3 Thu Dec 21 13:18:06 CET 2006 i686
GNU/Linux
Corruption is still present with the patch below (patched against
2.6.19.1 and removed task_io_account_cancelled_write call)
/Martin
[Not subscribed to the list]
> ---
> diff --git a/fs/buffer.c b/fs/buffer.c
> index d1f1b54..263f88e 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2834,7 +2834,7 @@ int try_to_free_buffers(struct page *page)
> int ret = 0;
>
> BUG_ON(!PageLocked(page));
> - if (PageWriteback(page))
> + if (PageDirty(page) || PageWriteback(page))
> return 0;
>
> if (mapping == NULL) { /* can this still happen? */
> @@ -2845,22 +2845,6 @@ int try_to_free_buffers(struct page *page)
> spin_lock(&mapping->private_lock);
> ret = drop_buffers(page, &buffers_to_free);
> spin_unlock(&mapping->private_lock);
> - if (ret) {
> - /*
> - * If the filesystem writes its buffers by hand (eg ext3)
> - * then we can have clean buffers against a dirty page. We
> - * clean the page here; otherwise later reattachment of buffers
> - * could encounter a non-uptodate page, which is unresolvable.
> - * This only applies in the rare case where try_to_free_buffers
> - * succeeds but the page is not freed.
> - *
> - * Also, during truncate, discard_buffer will have marked all
> - * the page's buffers clean. We discover that here and clean
> - * the page also.
> - */
> - if (test_clear_page_dirty(page))
> - task_io_account_cancelled_write(PAGE_CACHE_SIZE);
> - }
> out:
> if (buffers_to_free) {
> struct buffer_head *bh = buffers_to_free;
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index ed2c223..4f4cd13 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -176,7 +176,7 @@ static int hugetlbfs_commit_write(struct file *file,
>
> static void truncate_huge_page(struct page *page)
> {
> - clear_page_dirty(page);
> + cancel_dirty_page(page, /* No IO accounting for huge pages? */0);
> ClearPageUptodate(page);
> remove_from_page_cache(page);
> put_page(page);
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 4830a3b..350878a 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -253,15 +253,11 @@ static inline void SetPageUptodate(struct page
*page)
>
> struct page; /* forward declaration */
>
> -int test_clear_page_dirty(struct page *page);
> +extern void cancel_dirty_page(struct page *page, unsigned int
account_size);
> +
> int test_clear_page_writeback(struct page *page);
> int test_set_page_writeback(struct page *page);
>
> -static inline void clear_page_dirty(struct page *page)
> -{
> - test_clear_page_dirty(page);
> -}
> -
> static inline void set_page_writeback(struct page *page)
> {
> test_set_page_writeback(page);
> diff --git a/mm/memory.c b/mm/memory.c
> index c00bac6..79cecab 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1842,6 +1842,33 @@ void unmap_mapping_range(struct address_space
*mapping,
> }
> EXPORT_SYMBOL(unmap_mapping_range);
>
> +static void check_last_page(struct address_space *mapping, loff_t size)
> +{
> + pgoff_t index;
> + unsigned int offset;
> + struct page *page;
> +
> + if (!mapping)
> + return;
> + offset = size & ~PAGE_MASK;
> + if (!offset)
> + return;
> + index = size >> PAGE_SHIFT;
> + page = find_lock_page(mapping, index);
> + if (page) {
> + unsigned int check = 0;
> + unsigned char *kaddr = kmap_atomic(page, KM_USER0);
> + do {
> + check += kaddr[offset++];
> + } while (offset < PAGE_SIZE);
> + kunmap_atomic(kaddr,KM_USER0);
> + unlock_page(page);
> + page_cache_release(page);
> + if (check)
> + printk("%s: BADNESS: truncate check %u\n",
current->comm, check);
> + }
> +}
> +
> /**
> * vmtruncate - unmap mappings "freed" by truncate() syscall
> * @inode: inode of the file used
> @@ -1875,6 +1902,7 @@ do_expand:
> goto out_sig;
> if (offset > inode->i_sb->s_maxbytes)
> goto out_big;
> + check_last_page(mapping, inode->i_size);
> i_size_write(inode, offset);
>
> out_truncate:
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 237107c..b3a198c 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -845,38 +845,6 @@ int set_page_dirty_lock(struct page *page)
> EXPORT_SYMBOL(set_page_dirty_lock);
>
> /*
> - * Clear a page's dirty flag, while caring for dirty memory accounting.
> - * Returns true if the page was previously dirty.
> - */
> -int test_clear_page_dirty(struct page *page)
> -{
> - struct address_space *mapping = page_mapping(page);
> - unsigned long flags;
> -
> - if (!mapping)
> - return TestClearPageDirty(page);
> -
> - write_lock_irqsave(&mapping->tree_lock, flags);
> - if (TestClearPageDirty(page)) {
> - radix_tree_tag_clear(&mapping->page_tree,
> - page_index(page), PAGECACHE_TAG_DIRTY);
> - write_unlock_irqrestore(&mapping->tree_lock, flags);
> - /*
> - * We can continue to use `mapping' here because the
> - * page is locked, which pins the address_space
> - */
> - if (mapping_cap_account_dirty(mapping)) {
> - page_mkclean(page);
> - dec_zone_page_state(page, NR_FILE_DIRTY);
> - }
> - return 1;
> - }
> - write_unlock_irqrestore(&mapping->tree_lock, flags);
> - return 0;
> -}
> -EXPORT_SYMBOL(test_clear_page_dirty);
> -
> -/*
> * Clear a page's dirty flag, while caring for dirty memory accounting.
> * Returns true if the page was previously dirty.
> *
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 9bfb8e8..bf9e296 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -51,6 +51,20 @@ static inline void truncate_partial_page(struct
page *page, unsigned partial)
> do_invalidatepage(page, partial);
> }
>
> +void cancel_dirty_page(struct page *page, unsigned int account_size)
> +{
> + /* If we're cancelling the page, it had better not be mapped any
more */
> + if (page_mapped(page)) {
> + static unsigned int warncount;
> +
> + WARN_ON(++warncount < 5);
> + }
> +
> + if (TestClearPageDirty(page) && account_size)
> + task_io_account_cancelled_write(account_size);
> +}
> +
> +
> /*
> * If truncate cannot remove the fs-private metadata from the page,
the page
> * becomes anonymous. It will be left on the LRU and may even be
mapped into
> @@ -70,8 +84,8 @@ truncate_complete_page(struct address_space
*mapping, struct page *page)
> if (PagePrivate(page))
> do_invalidatepage(page, 0);
>
> - if (test_clear_page_dirty(page))
> - task_io_account_cancelled_write(PAGE_CACHE_SIZE);
> + cancel_dirty_page(page, PAGE_CACHE_SIZE);
> +
> ClearPageUptodate(page);
> ClearPageMappedToDisk(page);
> remove_from_page_cache(page);
> @@ -350,7 +364,6 @@ int invalidate_inode_pages2_range(struct
address_space *mapping,
> for (i = 0; !ret && i < pagevec_count(&pvec); i++) {
> struct page *page = pvec.pages[i];
> pgoff_t page_index;
> - int was_dirty;
>
> lock_page(page);
> if (page->mapping != mapping) {
> @@ -386,12 +399,8 @@ int invalidate_inode_pages2_range(struct
address_space *mapping,
> PAGE_CACHE_SIZE, 0);
> }
> }
> - was_dirty = test_clear_page_dirty(page);
> - if (!invalidate_complete_page2(mapping, page)) {
> - if (was_dirty)
> - set_page_dirty(page);
> + if (!invalidate_complete_page2(mapping, page))
> ret = -EIO;
> - }
> unlock_page(page);
> }
> pagevec_release(&pvec);
> -
> To unsubscribe from this list: send the line "unsubscribe
linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2006-12-21 14:07 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-21 13:27 mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3) Martin Johansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.