Messages in this thread | | | Subject | Re: Major 2.1.x problem index | From | (Eric W. Biederman) | Date | 21 Jun 1998 15:49:13 -0500 |
| |
>>>>> "PA" == H Peter Anvin <hpa@transmeta.com> writes:
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
| |