Messages in this thread Patch in this message | | | Subject | Re: [PATCH] per-backing dev unplugging #2 | From | Chris Mason <> | Date | Sun, 14 Mar 2004 15:43:53 -0500 |
| |
On Fri, 2004-03-12 at 15:51, Jens Axboe wrote: > On Fri, Mar 12 2004, Chris Mason wrote: > > On Fri, 2004-03-12 at 15:34, Jens Axboe wrote: > > > > > > > > I don't see how this can make too much of a difference, aside of perhaps > > > just moving the window a little. If page->mapping can disappear here, > > > that's still a possibility. > > > > As Andrew pointed out, the mapping struct won't disappear, but > > page->mapping may go null. So the idea is to use barriers to get a > > trusted copy of page->mapping, and use the copy everywhere. > > So trusting an atomic assignment of mapping = page->mapping, it should > work. It feels a bit icky, though.
I reproduced on 2.6.4-mm1 + backing dev, but 2.6.4-mm1 alone ran fine. To make a long story short, the swap address space and backing dev don't define an unplug_io_fn. I was able to reproduce quickly with a swap heavy workload. The patch below should fix the oops, but probably isn't correct solution since no queues will get unplugged while waiting on swap pages.
It keeps the barriers for reading page->mapping, I still think we need something there, maybe someone can suggest something better.
-chris
Index: linux.akpm/fs/buffer.c =================================================================== --- linux.akpm.orig/fs/buffer.c 2004-03-14 15:15:37.409241104 -0500 +++ linux.akpm/fs/buffer.c 2004-03-14 15:15:51.137154144 -0500 @@ -2923,7 +2923,10 @@ EXPORT_SYMBOL(try_to_free_buffers); int block_sync_page(struct page *page) { - blk_run_address_space(page->mapping); + struct address_space *mapping; + smp_mb(); + mapping = page->mapping; + blk_run_address_space(mapping); return 0; } Index: linux.akpm/mm/filemap.c =================================================================== --- linux.akpm.orig/mm/filemap.c 2004-03-14 15:15:32.876930120 -0500 +++ linux.akpm/mm/filemap.c 2004-03-14 15:15:51.138153992 -0500 @@ -120,8 +120,10 @@ void remove_from_page_cache(struct page static inline int sync_page(struct page *page) { - struct address_space *mapping = page->mapping; - + struct address_space *mapping; + + smp_mb(); + mapping = page->mapping; if (mapping && mapping->a_ops && mapping->a_ops->sync_page) return mapping->a_ops->sync_page(page); return 0; Index: linux.akpm/include/linux/blkdev.h =================================================================== --- linux.akpm.orig/include/linux/blkdev.h 2004-03-14 15:15:37.460233352 -0500 +++ linux.akpm/include/linux/blkdev.h 2004-03-14 15:17:24.035031520 -0500 @@ -527,7 +527,7 @@ static inline request_queue_t *bdev_get_ static inline void blk_run_backing_dev(struct backing_dev_info *bdi) { - if (bdi) + if (bdi && bdi->unplug_io_fn) bdi->unplug_io_fn(bdi); }
- 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/
| |