Re: Linus rollup

[Andrea Arcangeli <andrea@suse.de>]

On Wed, Jan 29, 2003 at 06:00:54PM -0800, Andrew Morton wrote:
> Andrea Arcangeli <andrea@suse.de> wrote:
> >
> > On Wed, Jan 29, 2003 at 05:27:43PM -0800, Andrew Morton wrote:
> > > @@ -82,11 +85,12 @@ static inline int fr_write_trylock(frloc
> > >  
> > >  	if (ret) {
> > >  		++rw->pre_sequence;
> > > -		wmb();
> > > +		mb();
> > >  	}
> > 
> > this isn't needed
> > 
> > 
> > if we hold the spinlock, the serialized memory can't be change under us,
> > so there's no need to put a read barrier, we only care that pre_sequence
> > is visible before the chagnes are visible and before post_sequence is
> > visible, hence only wmb() (after spin_lock and pre_sequence++) is
> > needed there and only rmb() is needed in the read-side.
> > 
> 
> OK, thanks muchly.
> 
> Lots more updates.  Here's the version which I currently have.  Looks like
> fr_write_lock() and fr_write_unlock() need to be switched back to rmb()?

you certainly mean wmb() not rmb(), right? If yes, then yes.

I actually didn't notice the write_begin/end, not sure who could need
them, I would suggest removing them, rather than to revert the mb()
there too.

> 
> 
> 
> #ifndef __LINUX_FRLOCK_H
> #define __LINUX_FRLOCK_H
> 
> /*
>  * Fast read-write spinlocks.
>  *
>  * Fast reader/writer locks without starving writers. This type of
>  * lock for data where the reader wants a consitent set of information
>  * and is willing to retry if the information changes.  Readers never
>  * block but they may have to retry if a writer is in
>  * progress. Writers do not wait for readers. 
>  *
>  * Generalization on sequence variables used for gettimeofday on x86-64 
>  * by Andrea Arcangeli
>  *
>  * This is not as cache friendly as brlock. Also, this will not work
>  * for data that contains pointers, because any writer could
>  * invalidate a pointer that a reader was following.
>  *
>  * Expected reader usage:
>  * 	do {
>  *	    seq = fr_read_begin();
>  * 	...
>  *      } while (seq != fr_read_end());
>  *
>  * On non-SMP the spin locks disappear but the writer still needs
>  * to increment the sequence variables because an interrupt routine could
>  * change the state of the data.
>  */
> 
> #include <linux/config.h>
> #include <linux/spinlock.h>
> #include <linux/preempt.h>
> 
> typedef struct {
> 	unsigned pre_sequence;
> 	unsigned post_sequence;
> 	spinlock_t lock;
> } frlock_t;
> 
> /*
>  * These macros triggered gcc-3.x compile-time problems.  We think these are
>  * OK now.  Be cautious.
>  */
> #define FR_LOCK_UNLOCKED { 0, 0, SPIN_LOCK_UNLOCKED }
> #define frlock_init(x)	do { *(x) = (frlock_t) FR_LOCK_UNLOCKED; } while (0)
> 
> /* Update sequence count only
>  * Assumes caller is doing own mutual exclusion with other lock
>  * or semaphore.
>  */
> static inline void fr_write_begin(frlock_t *rw)
> {
> 	preempt_disable();
> 	rw->pre_sequence++;
> 	mb();
> }
> 
> static inline void fr_write_end(frlock_t *rw)
> {
> 	mb();
> 	rw->post_sequence++;
> 	BUG_ON(rw->post_sequence != rw->pre_sequence);
> 	preempt_enable();
> }
> 
> /* Lock out other writers and update the count.
>  * Acts like a normal spin_lock/unlock.
>  */
> static inline void fr_write_lock(frlock_t *rw)
> {
> 	spin_lock(&rw->lock);
> 	rw->pre_sequence++;
> 	mb();
> }	
> 
> static inline void fr_write_unlock(frlock_t *rw) 
> {
> 	mb();
> 	rw->post_sequence++;
> 	spin_unlock(&rw->lock);
> }
> 
> static inline int fr_write_trylock(frlock_t *rw)
> {
> 	int ret = spin_trylock(&rw->lock);
> 
> 	if (ret) {
> 		++rw->pre_sequence;
> 		wmb();
> 	}
> 	return ret;
> }
> 
> static inline unsigned fr_read_begin(const frlock_t *rw) 
> {
> 	unsigned ret = rw->post_sequence;
> 	rmb();
> 	return ret;
> 	
> }
> 
> /* End of reader calculation -- fetch last writer start token */
> static inline unsigned fr_read_end(const frlock_t *rw)
> {
> 	rmb();
> 	return rw->pre_sequence;
> }
> 
> /*
>  * Possible sw/hw IRQ protected versions of the interfaces.
>  */
> #define fr_write_lock_irqsave(lock, flags)				\
> 	do { local_irq_save(flags);	fr_write_lock(lock); } while (0)
> #define fr_write_lock_irq(lock)						\
> 	do { local_irq_disable();	fr_write_lock(lock); } while (0)
> #define fr_write_lock_bh(lock)						\
>         do { local_bh_disable();	fr_write_lock(lock); } while (0)
> 
> #define fr_write_unlock_irqrestore(lock, flags)				\
> 	do { fr_write_unlock(lock); local_irq_restore(flags); } while(0)
> #define fr_write_unlock_irq(lock)					\
> 	do { fr_write_unlock(lock); local_irq_enable(); } while(0)
> #define fr_write_unlock_bh(lock)					\
> 	do { fr_write_unlock(lock); local_bh_enable(); } while(0)
> 
> #define fr_read_begin_irqsave(lock, flags)				\
> 	({ local_irq_save(flags);	fr_read_begin(lock); })
> 
> #define fr_read_end_irqrestore(lock, flags)				\
> 	({	unsigned ret = fr_read_end(lock);			\
> 		local_irq_save(flags);					\
> 		ret;							\
> 	})
> 
> #endif /* __LINUX_FRLOCK_H */
> 


Andrea
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/