> > Jan Kara <jack@xxxxxxx> writes: > > >> > > >> If jbd2_log_start_commit return 0 then it means that transaction > > >> was already committed. So we don't have to issue barrier for > > >> ordered mode, because it was already done during commit. > > > Umm, we have to - when a file has just been rewritten (i.e. no block > > > allocation), then i_datasync_tid is not updated and thus we won't commit > > > any transaction as a part of fdatasync (and that is correct because there > > > are no metadata that need to be written for that fdatasync). But we still > > > have to flush disk caches with data submitted by filemap_fdatawrite_and_wait. > > Yepp. I've missed that. i thought that transaction id updated even in that > > case. The most unpleasant part in ext4_sync_file implementation is that > > barrier is issued on each fsync() call. So some bad user may perform: > > while(1) fsync(fd); which result in bad system performance. And since barrier > > request is empty it is hard to detect the reason of troubles. > Actually, you'll be able to see the barrier requests in the blktrace dump > so it won't be that hard to detect. > > > Off course we may solve it by introducing some sort of dirty flag which is > > set in write_page, and clear in fsync. But it looks as ugly workaround. > I agree that sending barrier request on each fsync isn't very nice but > in common case, I'd assume that an application calls fsync only if it has > written something to the file previously. So I wouldn't invest much into > solving this until I see a realistic use case where it matters... > > > >> By unknown reason we ignored ret val from jbd2_log_wait_commit() > > >> so even in case of EIO fsync will succeed. > > > I just forgot jbd2_log_wait_commit can return a failure... > > In respect to previous comments the patch reduced to simple missed > > error check fix. > I guess you can resend the fix to Ted directly to catch his attention. > > > BTW: While investigating similar code in ext3 i've found what fsync is broken > > in case of external journal. > Yes, I've noticed this recently as well. So will you send a fix or should > I go and backport ext4 fixes of this? Oops, sorry, I've notice you sent the patches to the list already... Honza -- Jan Kara <jack@xxxxxxx> SuSE CR Labs -- 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
