On Thu 05-11-09 14:06:35, Jan Blunck wrote: > On Thu, Nov 05, Jan Kara wrote: > > > On Mon 02-11-09 11:04:45, Jan Blunck wrote: > > > The BKL is still used in ext2_put_super(), ext2_fill_super(), ext2_sync_fs() > > > ext2_remount() and ext2_write_inode(). From these calls ext2_put_super(), > > > ext2_fill_super() and ext2_remount() are protected against each other by > > > the struct super_block s_umount rw semaphore. The call in ext2_write_inode() > > > could only protect the modification of the ext2_sb_info through > > > ext2_update_dynamic_rev() against concurrent ext2_sync_fs() or ext2_remount(). > > > ext2_fill_super() and ext2_put_super() can be left out because you need a > > > valid filesystem reference in all three cases, which you do not have when > > > you are one of these functions. > > This is not quite true. Actually, ext2_sync_fs() is called also with > > s_umount held (for reading) so it cannot race with any of the mounting / > > umounting functions. But it should probably be guarded against running two > > instances of ext2_sync_fs() in parallel. > > Looking into the code, we probably have a race in ext2_write_inode that > > two threads can write one inode in parallel. But that's a separe issue from > > BKL removal. > > Have you actually seen that the previous patch introduces s_mutex? Meanwhile > that mutex has been replaced by a spinlock due to Andy's feedback. Ops, I wanted to comment on that patch as well but then decided it might be better to write my version and didn't get to it yet... Thinking more about it, probably go ahead with these two patches (modulo some changes I've now suggested in a reply to your second ext2 patch) and I can cleanup ext2 code after that. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- 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
