Messages in this thread | | | Date | Wed, 7 Jul 2004 14:30:15 -0700 | From | Andrew Morton <> | Subject | Re: Unnecessary barrier in sync_page()? |
| |
Andrea Arcangeli <andrea@suse.de> wrote: > > On Wed, Jul 07, 2004 at 04:57:04PM -0400, Chris Mason wrote: > > I wasn't worried about the locked bit when I added the barrier, my goal > > was to order things with people that set page->mapping to null. > > page->mapping cannot change from NULL to non-NULL there. > > it can only change from non-NULL to NULL, and there's no way to > serialize with the truncate without taking the page lock.
And we cannot lock the page because, err, we need to run sync_page() for that.
> The one extremely important fix you did around the same time, has been > to "cache" the value of "mapping" in the kernel stack, so that it > remains the same during the while function (so that it cannot start > non-NULL an finish NULL).
But the page can come unlocked and truncate or page reclaim can remove the page from the mapping and memory reclaim can reclaim the inode:
int block_sync_page(struct page *page) { struct address_space *mapping;
smp_mb(); mapping = page_mapping(page); -> right here if (mapping) -> go boom here blk_run_backing_dev(mapping->backing_dev_info, page); return 0; }
But I cannot think of any callers of sync_page() who don't have a ref on the inode, so... - 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/
| |