linux-kernel - RFC: one-bit mutexes (was: Re: [PATCH 2/3] Memory management livelock)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 5 Oct 2008 18:11:54 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
cc:	linux-kernel@...r.kernel.org, agk@...hat.com, mbroz@...hat.com,
	chris@...chsys.com
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@...hat.com> wrote:
> 
> > Avoid starvation when walking address space.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@...hat.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.
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/