On Tue 09-10-12 14:07:52, Fernando Luis Vazquez Cao wrote:
> >>diff -urNp linux-3.6.0-rc7-orig/fs/buffer.c linux-3.6.0-rc7/fs/buffer.c
> >>--- linux-3.6.0-rc7-orig/fs/buffer.c 2012-09-26 13:20:14.842365056 +0900
> >>+++ linux-3.6.0-rc7/fs/buffer.c 2012-09-26 15:02:22.630595704 +0900
...
> >What I would put here is:
> > if (!res) {
> > deactivate_locked_super(sb);
> > /*
> > * We have to re-acquire s_umount because
> > * iterate_supers_write() will unlock it. It still holds
> > * passive reference so sb cannot be freed under us.
> > */
> > down_write(&sb->s_umount);
> > }
> >
> >Is there any problem with this I miss?
>
> The reason I wrote the code as I did is that I did not want to re-acquire
> s_umount in the normal case (s_active >= 2 entering the if statement).
>
> What about combining our approaches and doing something like this?:
>
> if (!res && !atomic_add_unless(&sb->s_active, -1, 1)) {
> deactivate_locked_super(sb);
> down_write(&sb->s_umount);
> }
Well, we are speaking about emergency thaw code. That's no performance
critical path by any means so I'd trade code simplicity for speed as much
as possible. atomic_add_unless() makes you think twice what the hell we are
doing here so I'd avoid it...
Honza
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
