On Mon 29-01-18 11:11:23, Chengguang Xu wrote: > Currently syncfs(2) call on overlayfs just simply call sync_filesystem() > on upper_sb to synchronize whole dirty inodes in upper filesystem > regardless of the overlay ownership of the inode. In the use case of > container, when multiple containers using the same underlying upper > filesystem, it has some shortcomings as below. > > (1) Performance > Synchronization is probably heavy because it actually syncs unnecessary > inodes for target overlayfs. > > (2) Interference > Unplanned synchronization will probably impact IO performance of > unrelated container processes on the other overlayfs. > > This patch iterates upper inode list in overlayfs to only sync target > dirty inodes and wait for completion. By doing this, It is able to reduce > cost of synchronization and will not seriously impact IO performance of > unrelated processes. In special case, when having very few dirty inodes > and a lot of clean upper inodes in overlayfs, then iteration may waste > time so that synchronization is slower than before, but we think the > potential for performance improvement far surpass the potential for > performance regression in most cases. > > Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> Just one nit below. With that fixed you can add: Reviewed-by: Jan Kara <jack@xxxxxxx> > + > +next: > + if (cond_resched()) > + blk_start_plug(&plug); This is wrong. You cannot start plug for a second time. Plugs are completely seemless wrt scheduling points (places where process can sleep). And cond_resched() is just an explicit scheduling point but e.g. taking a lock is an implicit scheduling point as well. So just do cond_resched() here and remove the blk_start_plug() call. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
