2018-03-27 19:36 GMT+08:00 Brian Foster <bfoster@xxxxxxxxxx>: > On Mon, Mar 26, 2018 at 05:55:26PM -0700, Shaohua Li wrote: >> On Mon, Mar 26, 2018 at 12:28:31PM -0400, Brian Foster wrote: >> > On Mon, Mar 26, 2018 at 08:59:04AM +1100, Dave Chinner wrote: >> > > On Fri, Mar 23, 2018 at 10:24:03PM +0800, 张本龙 wrote: >> > > > Hi Shaohua and XFS, >> > > > >> > > > May I ask how are we gonna handle REQ_META issued from XFS? As you >> > > > mentioned about charging to root cgroup (also in an earlier email >> > > > discussion), and seems the 4.16.0-rc6 code is not handling it >> > > > separately. >> > > > >> > > > In our case to support XFS cgroup writeback control, which was ported >> > > > and slightly adapted to 3.10.0, ignoring xfs log bios resulted in >> > > > trouble. Threads from throttled docker might submit_bio in following >> > > > path by its own identity, this docker blkcg accumulated large amounts >> > > > of data (e.g., 20GB), thus such log gets blocked. >> > > >> > > And thus displaying the reason why I originally refused to merge >> > > this code until regression tests were added to fstests to exercise >> > > these sorts of issues. This stuff adds new internal filesystem IO >> > > ordering constraints, so we need tests that exercise it and ensure >> > > we don't accidentally break it in future. >> > > >> > >> > Hmm, but if the user issues fsync from the throttled cgroup then won't >> > that throttling occur today, regardless of cgroup aware writeback? My >> > understanding is that cgawb just accurately accounts writeback I/Os to >> > the owner of the cached pages. IOW, if the buffered writer and fsync >> > call are in the same throttled cgroup, then the throttling works just as >> > it would with cgawb and the writer being in a throttled cgroup. >> > >> > So ISTM that this is an independent problem. What am I missing? >> > >> > Shaohua, >> > >> > Do you have a reference to the older metadata related patch mentioned in >> > the commit log that presumably addressed this? >> >> The problem is about priority reversion. Say you do a fsync in a low prio >> cgroup, the IO will be submitted with low prio. Now you do a fsync in a high >> prio cgroup, the cgroup will wait for fsync IO finished, which is already >> submitted by the low prio cgroup and run in low prio. This makes the high prio >> cgroup run slow. The proposed patch is to force all metadata write submitted >> from root cgroup regardless which task submitted, which can fix this issue. >> > > Right, but it seems to me that this can happen with or without cgroup > aware writeback. This patch just introduces the final bits required to > carry the page owner from however it is tracked in the writeback machine > to the bio submitted by the fs. It doesn't introduce/enable/implement > I/O throttling itself, which is already in place and usable (outside of > the buffered write page owner problem fixed by this patch), right? > > So without this patch, if a task in throttled cgroup A does a bunch of > buffered writes and calls fsync, then another task in unthrottled cgroup > B calls fsync, aren't we (XFS) susceptible to priority inversion via > these same log I/O serialization points? If not, then what am I missing? > Ok first let's agree we're not talking about 2 groups fsync the same file. What's demonstrated in the original post, is that group-A MIGHT submit xlog in the fsync path via xlog_sync() -> xfs_buf_submit(). Threads from group-B are waiting for the log from flush_work(), at the same time kworkers from xlog_cil_push_work(). The problem is not about fsync get stuck on the target file. Actually group-B should be waiting on filemap_write_and_wait_range() if it were, as xfs_file_fsync() would flush the real data before _xfs_log_force_lsn. Here is the setup: group-A has 10 fio jobs each running on 20GB files, and also some agents with not much IO; group-B just has the agents. With this patch we set a bps=20Mb/s to A, thus the large amounts of fio traffic are blocking the group (Note the fio are not doing any fsync though). At this time, one agent does an fsync to a non-dirty file, bypassing filemap_write_and_wait_range() and doing xlog_sync(). Here we go, log bios are stuck in group-A. Then group-B and kworkers are waiting for log integrity indefinitely from various paths. Without this patch fio should writeout at full disk speed, say 200MB/s, so it's not accumulated in group-A. > I'm not saying this isn't a problem that needs fixing, I just want to > make sure I understand the fundamental problem(s), what this cgawb patch > actually does and doesn't do and whether there is a logical dependency > between this patch and the proposed metadata filtering patch. > > Brian > >> Thanks, >> Shaohua >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
