Jan Kara <jack@xxxxxxx> writes:
>> + /* workqueue for aio+dio+o_sync disk cache flushing */
>> + struct workqueue_struct *aio_dio_flush_wq;
>> +
> Hmm, looking at the patch I'm wondering why did you introduce the new
> workqueue? It seems dio_unwritten_wq would be enough? You just need to
> rename it to something more appropriate ;)
I used a new workqueue as the operations are blocking, and I didn't want
to hold up other progress. If you think re-using the unwritten_wq is
the right thing to do, I'm happy to comply.
>> + /*
>> + * This function has two callers. The first is the end_io_work
>> + * routine just below. This is an asynchronous completion context.
>> + * The second is in the fsync path. For the latter path, we can't
>> + * return from here until the job is done. Hence, we issue a
>> + * blocking blkdev_issue_flush call.
>> + */
>> + if (io->flag & EXT4_IO_END_NEEDS_SYNC) {
>> + /*
>> + * Ideally, we'd like to know if the force_commit routine
>> + * actually did send something to disk. If it didn't,
>> + * then we need to issue the cache flush by hand. For now,
>> + * play it safe and do both.
>> + */
>> + ret = ext4_force_commit(inode->i_sb);
>> + if (ret)
>> + goto endio;
>> + ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_NOIO, NULL);
> Look at what ext4_sync_file() does. It's more efficient than this.
> You need something like:
> commit_tid = file->f_flags & __O_SYNC ? EXT4_I(inode)->i_sync_tid :
> EXT4_I(inode)->i_datasync_tid;
> if (journal->j_flags & JBD2_BARRIER &&
> !jbd2_trans_will_send_data_barrier(journal, commit_tid))
> needs_barrier = true;
> jbd2_log_start_commit(journal, commit_tid);
> jbd2_log_wait_commit(journal, commit_tid);
> if (needs_barrier)
> blkdev_issue_flush(inode->i_sb->s_bdev, GFP_NOIO, NULL);
Great, thanks for the pointer!
Cheers,
Jeff
--
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
