Messages in this thread | | | Date | Sun, 5 Oct 2008 18:11:54 -0400 (EDT) | From | Mikulas Patocka <> | Subject | RFC: one-bit mutexes (was: Re: [PATCH 2/3] Memory management livelock) |
| |
Hi
I removed the repeated code and create a new bit mutexes. They are space-efficient mutexes that consume only one bit. See the next 3 patches.
If you are concerned about the size of an inode, I can convert other mutexes to bit mutexes: i_mutex and inotify_mutex. I could also create bit_spinlock (one-bit spinlock that uses test_and_set_bit) and save space for address_space->tree_lock, address_space->i_mmap_lock, address_space->private_lock, inode->i_lock.
Look at it and say what you think about the idea of condensing mutexes into single bits.
Mikulas
> > > Subject: [PATCH 2/3] Memory management livelock > > Please don't send multiple patches with identical titles - think up a > good, unique, meaningful title for each patch. > > On Wed, 24 Sep 2008 14:52:18 -0400 (EDT) Mikulas Patocka <mpatocka@redhat.com> wrote: > > > Avoid starvation when walking address space. > > > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > > > Please include a full changelog with each iteration of each patch. > > That changelog should explain the reason for playing games with > bitlocks so Linus doesn't have kittens when he sees it. > > > include/linux/pagemap.h | 1 + > > mm/filemap.c | 20 ++++++++++++++++++++ > > mm/page-writeback.c | 37 ++++++++++++++++++++++++++++++++++++- > > mm/truncate.c | 24 +++++++++++++++++++++++- > > 4 files changed, 80 insertions(+), 2 deletions(-) > > > > Index: linux-2.6.27-rc7-devel/include/linux/pagemap.h > > =================================================================== > > --- linux-2.6.27-rc7-devel.orig/include/linux/pagemap.h 2008-09-24 02:57:37.000000000 +0200 > > +++ linux-2.6.27-rc7-devel/include/linux/pagemap.h 2008-09-24 02:59:04.000000000 +0200 > > @@ -21,6 +21,7 @@ > > #define AS_EIO (__GFP_BITS_SHIFT + 0) /* IO error on async write */ > > #define AS_ENOSPC (__GFP_BITS_SHIFT + 1) /* ENOSPC on async write */ > > #define AS_MM_ALL_LOCKS (__GFP_BITS_SHIFT + 2) /* under mm_take_all_locks() */ > > +#define AS_STARVATION (__GFP_BITS_SHIFT + 3) /* an anti-starvation barrier */ > > > > static inline void mapping_set_error(struct address_space *mapping, int error) > > { > > Index: linux-2.6.27-rc7-devel/mm/filemap.c > > =================================================================== > > --- linux-2.6.27-rc7-devel.orig/mm/filemap.c 2008-09-24 02:59:33.000000000 +0200 > > +++ linux-2.6.27-rc7-devel/mm/filemap.c 2008-09-24 03:13:47.000000000 +0200 > > @@ -269,10 +269,19 @@ int wait_on_page_writeback_range(struct > > int nr_pages; > > int ret = 0; > > pgoff_t index; > > + long pages_to_process; > > > > if (end < start) > > return 0; > > > > + /* > > + * Estimate the number of pages to process. If we process significantly > > + * more than this, someone is making writeback pages under us. > > + * We must pull the anti-starvation plug. > > + */ > > + pages_to_process = bdi_stat(mapping->backing_dev_info, BDI_WRITEBACK); > > + pages_to_process += (pages_to_process >> 3) + 16; > > This sequence appears twice and it would probably be clearer to > implement it in a well-commented function. > > > pagevec_init(&pvec, 0); > > index = start; > > while ((index <= end) && > > @@ -288,6 +297,10 @@ int wait_on_page_writeback_range(struct > > if (page->index > end) > > continue; > > > > + if (pages_to_process >= 0) > > + if (!pages_to_process--) > > + wait_on_bit_lock(&mapping->flags, AS_STARVATION, wait_action_schedule, TASK_UNINTERRUPTIBLE); > > This is copied three times and perhaps also should be factored out. > > Please note that an effort has been made to make mm/filemap.c look > presentable in an 80-col display. > > > wait_on_page_writeback(page); > > if (PageError(page)) > > ret = -EIO; > > @@ -296,6 +309,13 @@ int wait_on_page_writeback_range(struct > > cond_resched(); > > } > > > > + if (pages_to_process < 0) { > > + smp_mb__before_clear_bit(); > > + clear_bit(AS_STARVATION, &mapping->flags); > > + smp_mb__after_clear_bit(); > > + wake_up_bit(&mapping->flags, AS_STARVATION); > > + } > > This sequence is repeated three or four times and should be pulled out > into a well-commented function. That comment should explain the logic > behind the use of these barriers, please. > >
| |