On 2012/10/09 17:20, Jan Kara wrote:
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...
I left (and documented) atomic_add_unless() in the new iteration I
cooked locally,
but I can get rid of it if you feel strongly about it.
I really appreciate you in-depth comments!
Thanks,
Fernando
--
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
