PA> Followup to: <m11zskwucz.fsf@flinx.npwt.net>
PA> By author: ebiederm+eric@npwt.net (Eric W. Biederman)
PA> In newsgroup: linux.dev.kernel
>>
>> I sent mail to H. Peter Anvin <hpa@zytor.com> who I believe is
>> responsible to:
>> a) see if I could understand the change in 2.1.93 (where this
>> appeared) The moving of the super block locking puzzles me a lot.
>> b) to hopefully coordinate a fix for this problem.
>>
>> So far he hasn't replied. And I'm not totally confident I can fix the
>> code right until I understand why we moved the lock super calls. If I
>> don't get some feed back soon I'll try anyway.
>>
PA> I'm not responsible to make you understand anything. I'm trying to
PA> coordinate a fix for it, on the other hand, but I have been on
PA> vacation.
Oops I used bad punctionation. I meant:
I sent may to you (who I believe is responsible for this code).
- To inform you of the bug,
- To help get the coordination going to fix the problem
- To ask and to ask about the move of lock_super. Which was part of
the change where the bug was introduced, and seems important to
understand to fix the problem correctly. Since when I don't
understand code I try to tread very lightly.
All I have been able to figure out about the lock_super move is that
it would appear to help reduce mount/umount races if there weren't a
semaphore over both the mount and unmount operations currently.
If that is indeed the reason for the change I would suggest code of
the following form:
retval = d_umount(sb);
if (retval)
goto out;
/* Release any cached inodes */
if (sb->s_op && sb->s_op->uncache_inodes) {
sb->s_op->uncache_inodes(sb);
}
/* Forget any remaining inodes */
if (invalidate_inodes(sb)) {
printk("VFS: Busy inodes after unmount. \n");
}
if (sb->s_op) {
if (sb->s_op->write_super && sb->s_dirt)
sb->s_op->write_super(sb);
}
lock_super(sb);
if (sb->s_op) {
if (sb->s_op->put_super)
sb->s_op->put_super(sb);
}
sb->s_dev = 0; /* Free the superblock */
unlock_super(sb);
----
With the uncache_inodes and the invalidate_inodes coming before the
write_super, just in case they update the super_block.
To reduce the race with mount, if we need to do more than the current
semaphore allows I would add an explicit unmounting state to super
blocks.
And have read_super fail if it get_super succeeded instead of reusing
the super block which is wrong, but can't happen currently.
Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu