lkml.org 
[lkml]   [2008]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRFC: 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.
>
>


\
 
 \ /
  Last update: 2008-10-06 00:15    [W:0.094 / U:2.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site