lkml.org 
[lkml]   [2004]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] anobjrmap 9 priority mjb tree
On Wed, Apr 14, 2004 at 11:40:52PM -0400, Rajesh Venkatasubramanian wrote:
>
> > > 2) However, vmas can be added and removed from a vm_set list
> > > by just holding the read lock and a bit lock (vm_set_lock)
> > > in the corresponding prio_tree node.
> >
> > no way, you cannot bitflip vm_flags unless you own the mmap_sem, this
> > patch seems very broken to me, it should randomly corrupt memory in
> > vma->vm_flags while racing against mprotect etc.. or am I missing
> > something?
>
> I don't know why bit_spin_lock with vma->vm_flags should be a problem
> if it is used without mmap_sem. Can you explain ?

you seem not to know all rules about the atomic operations in smp, you
cannot just set_bit on one side and use non-atomic operations on the
other side, and expect the set_bit not to invalidate the non-atomic
operations.

The effect of the mprotect may be deleted by your new concurrent
set_bit and stuff like that.

> If it is really racy to use bit_spin_lock on vm_flags without mmap_sem

it __definitely__ is racy.

> Well. In that case, we can use rwsem as you mentioned below: take
> down_write on all modifications and take down_read on pageout. Here, you

exactly, this also avoids the more complex (and racy, but the racy would
be easy to fix by adding another 4 atomic bytes to the vma) double
locking that you introduced.

> allow multiple parallel page_referenced and try_to_unmap on the same
> inode, however with only one modification at a time.

exactly.

> Wherease my solution will allow multiple modifications at the same
> time (if possible) with only one pageout routine at a time. I chose
> this solution because Martin's SDET took big hit in common cases of
> adding and removing vmas from the i_mmap{_shared} data structure.

you can still fix the smp race condition by trivially adding 4 bytes to
the vma (i.e. a vma->vm_flags_atomic), but I'd be surprised if your
double locking actually improve things, Martin is running on a very
parallel old-numa where cacheline bouncing across nodes pass through a
fibre channel IIRC, and the cacheline bouncing that the semaphore
generates is huge, it's not necessairly huge contention, it's just the
bouncing that hurts, and the down_read won't help at all for the
cacheline trashing, it'll still bounce like before. Though you may gain
something minor, but I doubt it'd be huge.

I'd suggest Martin to give a try to the racy code, it's just good enough
for a pratical experiment (the race shouldn't easily trigger so it
probably passes one run of SDET safely).
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:02    [W:0.134 / U:0.764 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site