lkml.org 
[lkml]   [2010]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 2/3] Call the filesystem back whenever a page is removed from the page cache
    On Wed, 1 Dec 2010, Trond Myklebust wrote:
    >
    > --- a/mm/truncate.c
    > +++ b/mm/truncate.c
    > @@ -108,6 +108,10 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
    > clear_page_mlock(page);
    > remove_from_page_cache(page);
    > ClearPageMappedToDisk(page);
    > +
    > + if (mapping->a_ops->freepage)
    > + mapping->a_ops->freepage(page);
    > +
    > page_cache_release(page); /* pagecache ref */
    > return 0;
    > }

    I think Linus recommended that one be done in remove_from_page_cache()
    to catch all instances: did that get overruled later for some reason?

    > --- a/mm/vmscan.c
    > +++ b/mm/vmscan.c
    > @@ -454,6 +454,7 @@ static int __remove_mapping(struct address_space *mapping, struct page *page)
    > BUG_ON(!PageLocked(page));
    > BUG_ON(mapping != page_mapping(page));
    >
    > + preempt_disable();
    > spin_lock_irq(&mapping->tree_lock);
    > /*
    > * The non racy check for a busy page.
    > @@ -492,10 +493,19 @@ static int __remove_mapping(struct address_space *mapping, struct page *page)
    > swp_entry_t swap = { .val = page_private(page) };
    > __delete_from_swap_cache(page);
    > spin_unlock_irq(&mapping->tree_lock);
    > + preempt_enable();
    > swapcache_free(swap, page);
    > } else {
    > + void (*freepage)(struct page *);
    > +
    > + freepage = mapping->a_ops->freepage;
    > +
    > __remove_from_page_cache(page);
    > spin_unlock_irq(&mapping->tree_lock);
    > + if (freepage != NULL)
    > + freepage(page);
    > + preempt_enable();
    > +
    > mem_cgroup_uncharge_cache_page(page);
    > }
    >
    > @@ -503,6 +513,7 @@ static int __remove_mapping(struct address_space *mapping, struct page *page)
    >
    > cannot_free:
    > spin_unlock_irq(&mapping->tree_lock);
    > + preempt_enable();
    > return 0;
    > }

    I took his "stop_machine()" explanation ("an idle period for everything.
    And just a preemption reschedule isn't enough for that") to imply that
    there's no need for your preempt_disable/preempt_enable there: they
    don't add anything to the module unload case, and they don't help the
    spin_unlock_irq issue (and you're already being rightly careful to note
    freepage in advance).

    But maybe I misunderstood.

    Hugh


    \
     
     \ /
      Last update: 2010-12-02 04:37    [W:0.024 / U:3.204 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site