On Tue 02-03-10 13:01:52, Dmitry Monakhov wrote: > Christoph Hellwig <hch@xxxxxx> writes: > >> Al, Christoph, do I miss something or there is really nothing which > >> prevents a process from opening a file after the fs_may_remount_ro() check > >> in do_remount_sb()? > > > > No, there is nothing. We really do need a multi-stage remount read-only > > process: > > > > 1) stop any writes from userland, that is opening new files writeable > This is not quite good idea because sync may take really long time, > #fsstress -p32 -d /mnt/TEST -l9999999 -n99999999 -z -f creat=100 -f write=100 > #sleep 60; > #killall -9 fsstress > #time mount mnt -oremount,ro > it take several minutes to complete. > And at the end it may fail but other reason. Two points here: 1) Current writeback code has a bug that while we are umounting/remounting, sync_filesystem() just degrades to doing all writeback in sync mode (because any non-sync writeback fails to get s_umount sem for reading and thus skips all the inodes of the superblock). This has considerable impact on the speed of sync during umount / remount. 2) IMHO it's not bad to block all opens for writing during remounting RO (and thus also during the sync). It's not a performance issue (remounting RO does not happen often), it won't confuse any application or so even if we later decide we cannot really finish remounting. Surely we'd have to come up with a better waiting scheme than just cpu_relax() in mnt_want_write() but that shouldn't be hard. The only thing I'm slightly worried about is whether we won't hit some locking issues (i.e., caller of mnt_want_write holding some lock needed to finish remount...). > > 2) stop any periodic writeback from the VM or filesystem-internal > > 3) write out all filesystem data and metadata > > 4) mark the filesystem fully read-only > > I've tried to sole the issue in lightly another way > Please take a look on this > http://marc.info/?l=linux-fsdevel&m=126723036525624&w=2 > 1) Mark fs as GOING_TO_REMOUNT > 2) any new writer will clear this flag > This allow us to not block > 3) check flag before fssync and after and return EBUSY in this case. > 4) At this time we may to block writers (this is absent in my patch) > It is acceptable to block writers at this time because later stages > doesn't take too long. > 5) perform fs-specific remount method. > 6) Marks filesystem as MS_RDONLY. I like my solution more since in my solution, admin does not have go hunting for an application which keeps touching the filesystem while he is trying to remount it read only (currently, using lsof is usually enough but after your changes, running something like "while true; do touch /mnt/; done" has much larger window to stop remounting RO). But in principle your solution is acceptable for me as well. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
