On Sat, Jan 02, 2010 at 06:43:12PM +0100, Andi Kleen wrote:> > I only have reiserfs partitions in my laptop and my testbox,> > nothing else. And that because I'm now maintaining it de facto.> > AFAIK it's widely used in SUSE installations. It was the default> for a long time.> > And right now as in 2.6.32 it's in a state of> "may randomly explode/deadlock". And no clear path out of it. Not good.> > I am very concerned about destabilizing a widely used file system> like this. This has the potential to really hurt users.

I understand your worries. And I've been very cautious with that,waiting for three cycles before requesting an upstream merge. I didit because the isolated tree model did not scale anymore.

Now that it's upstream, I get more testing and I expect that, inthe end of this cycle, I get most of these issues reported andfixed.

Serious users who run serious datas won't ship 2.6.33, they will shipa further stable version 2.6.33.x (if they haven't converted theirfilesystems already).And at this time, things should be 99% fixed.

> > - that would require a notifier in schedule(), one notifier> > per sub-bkl. That's horrible for performances. And for> > the scheduler. I will be the first to NAK.> > I thought the original idea was to find everything that > can sleep in reiserfs and simply wrap it with lock dropping?>> That should be roughly equivalent to the old BKL semantics.>> Where did it go wrong?

That's the theory. Fitting into this strict scheme brings performanceregressions. The bkl is a spinlock, it disables preemption, it isrelaxed on sleep, and doesn't have locking dependencies. Moreoverit's not a lock but a simulation of a NO_PREEMPT UP flow, with allthe fixup guardians that come with (fixup if we schedule, asscheduling brings races).

From the conversion is borned a mutex. Even though we haveadaptive spinning, we don't catch up spinlock performancesas it's not a pure optimized looping fast path, and it mayactually just sleep.

The bkl is relaxed only when we sleep. Now simulating that witha mutex that gets explicitly relaxed is not the same thing aswe need to relax the lock each time we _might_ sleep. It meanswe relax more and that brings performance regressions.

That said it's sometimes a drawback for the bkl to be relaxedevery time we schedule, because we need to fixup after that,sometimes we need to re-walk into the entire tree, etc...

So sometimes we can do better. There are some places wherewe don't relax like did the bkl, so that we don't need to fixup,and we get a win of performances.

You see? The bkl semantics must not be always strictly imitated onsuch conversion. It depends on what does the code. In reiserfs,sometimes it was desired that the bkl get relaxed, sometimes itwasn't. And all the reiserfs code deals with that.

With a mutex we have the choice. So the conversion has beena balance between performance regressions brought by the mutexconversion, and the performance win because we have actuallymore control with a traditional lock.

That said there are places where we really need to sleep, likewhen we grab another lock, so that we don't create inverteddependencies.

That said, if the general opinion is in favour of unmergingthe bkl removal changes in reiserfs. Then please do.

Just to express my point of view, as my primary goal is notto fix reiserfs but the kernel: If you are afraid of suchchanges, your kernel will just become mildewed by the time.You need to drop such bad ill-legacies if you want it toevolve. Until every users of the big kernel lock will remainin the kernel, vanilla upstream will keep it as ball and chain,won't ever be able to perform any serious real time service,etc...

So yes this is risky. But I think this is necessary. And as Iexplained above, things will be fine as serious datas are notmanipulated with a random -rc2 (except my own datas...).