lkml.org 
[lkml]   [2006]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: 2.6.19 file content corruption on ext3
    From
    Date
    On Tue, 2006-12-19 at 15:36 +1100, Nick Piggin wrote:

    > plain text document attachment (fs-fix.patch)
    > Index: linux-2.6/fs/buffer.c
    > ===================================================================
    > --- linux-2.6.orig/fs/buffer.c 2006-12-19 15:15:46.000000000 +1100
    > +++ linux-2.6/fs/buffer.c 2006-12-19 15:36:01.000000000 +1100
    > @@ -2852,7 +2852,17 @@ int try_to_free_buffers(struct page *pag
    > * This only applies in the rare case where try_to_free_buffers
    > * succeeds but the page is not freed.
    > */
    > - clear_page_dirty(page);
    > +
    > + /*
    > + * If the page has been dirtied via the user mappings, then
    > + * clean buffers does not indicate the page data is actually
    > + * clean! Only clear the page dirty bit if there are no dirty
    > + * ptes either.
    > + *
    > + * If there are dirty ptes, then the page must be uptodate, so
    > + * the above concern does not apply.
    > + */
    > + clear_page_dirty_sync_ptes(page);
    > }
    > out:
    > if (buffers_to_free) {
    > Index: linux-2.6/include/linux/page-flags.h
    > ===================================================================
    > --- linux-2.6.orig/include/linux/page-flags.h 2006-12-19 15:17:18.000000000 +1100
    > +++ linux-2.6/include/linux/page-flags.h 2006-12-19 15:34:24.000000000 +1100
    > @@ -254,6 +254,7 @@ static inline void SetPageUptodate(struc
    > struct page; /* forward declaration */
    >
    > int test_clear_page_dirty(struct page *page);
    > +int test_clear_page_dirty_sync_ptes(struct page *page);
    > int test_clear_page_writeback(struct page *page);
    > int test_set_page_writeback(struct page *page);
    >
    > @@ -262,6 +263,11 @@ static inline void clear_page_dirty(stru
    > test_clear_page_dirty(page);
    > }
    >
    > +static inline void clear_page_dirty_sync_ptes(struct page *page)
    > +{
    > + test_clear_page_dirty_sync_ptes(page);
    > +}
    > +
    > static inline void set_page_writeback(struct page *page)
    > {
    > test_set_page_writeback(page);
    > Index: linux-2.6/mm/page-writeback.c
    > ===================================================================
    > --- linux-2.6.orig/mm/page-writeback.c 2006-12-19 15:17:53.000000000 +1100
    > +++ linux-2.6/mm/page-writeback.c 2006-12-19 15:33:29.000000000 +1100
    > @@ -844,9 +844,10 @@ EXPORT_SYMBOL(set_page_dirty_lock);
    >
    > /*
    > * Clear a page's dirty flag, while caring for dirty memory accounting.
    > + * Does not clear pte dirty bits.
    > * Returns true if the page was previously dirty.
    > */
    > -int test_clear_page_dirty(struct page *page)
    > +static int test_clear_page_dirty_leave_ptes(struct page *page)
    > {
    > struct address_space *mapping = page_mapping(page);
    > unsigned long flags;
    > @@ -862,10 +863,8 @@ int test_clear_page_dirty(struct page *p
    > * 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);
    > + if (mapping_cap_account_dirty(mapping))
    > dec_zone_page_state(page, NR_FILE_DIRTY);
    > - }
    > return 1;
    > }
    > write_unlock_irqrestore(&mapping->tree_lock, flags);
    > @@ -873,9 +872,43 @@ int test_clear_page_dirty(struct page *p
    > }
    > return TestClearPageDirty(page);
    > }
    > +
    > +/*
    > + * As above, but does clear dirty bits from ptes
    > + */
    > +int test_clear_page_dirty(struct page *page)
    > +{
    > + struct address_space *mapping = page_mapping(page);
    > +
    > + if (test_clear_page_dirty_leave_ptes(page)) {
    > + if (mapping_cap_account_dirty(mapping))
    > + page_mkclean(page);
    > + return 1;
    > + }
    > + return 0;
    > +}
    > EXPORT_SYMBOL(test_clear_page_dirty);
    >
    > /*
    > + * As above, but redirties page if any dirty ptes are found (and then only
    > + * if the mapping accounts dirty pages, otherwise dirty ptes are left dirty
    > + * but the page is cleaned).
    > + */
    > +int test_clear_page_dirty_sync_ptes(struct page *page)
    > +{
    > + struct address_space *mapping = page_mapping(page);
    > +
    > + if (test_clear_page_dirty_leave_ptes(page)) {
    > + if (mapping_cap_account_dirty(mapping)) {
    > + if (page_mkclean(page))
    > + set_page_dirty(page);
    > + }
    > + return 1;
    > + }
    > + return 0;
    > +}
    > +
    > +/*
    > * Clear a page's dirty flag, while caring for dirty memory accounting.
    > * Returns true if the page was previously dirty.
    > *

    Hmm, not quite; It certainly look better than the extra ,[01] tagged to
    test_clear_page_dirty() though. Although I would have expected it the
    other way around - test_clear_pages_dirty_sync_ptes to be the default
    case and test_clear_pages_dirty_clean_ptes to be used in
    clear_page_dirty_for_io().

    Anyway it has the same issues as the others. See what happens when you
    run two test_clear_page_dirty_sync_ptes() consecutively, you still loose
    PG_dirty even though the page might actually be dirty.



    -
    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/

    \
     
     \ /
      Last update: 2006-12-19 08:27    [W:0.032 / U:0.348 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site