On Wed, Mar 23, 2022 at 06:11:03AM +0000, CGEL wrote: > On Tue, Mar 22, 2022 at 09:27:58AM -0400, Johannes Weiner wrote: > > On Tue, Mar 22, 2022 at 02:47:42AM +0000, CGEL wrote: > > > On Mon, Mar 21, 2022 at 10:33:20AM -0400, Johannes Weiner wrote: > > > > On Wed, Mar 16, 2022 at 06:39:28AM +0000, cgel.zte@xxxxxxxxx wrote: > > > > > From: Yang Yang <yang.yang29@xxxxxxxxxx> > > > > > > > > > > psi tracks the time spent on submitting the IO of refaulting file pages > > > > > and anonymous pages[1]. But after we tracks refaulting anonymous pages > > > > > in swap_readpage[2][3], there is no need to track refaulting anonymous > > > > > pages in submit_bio. > > > > > > > > > > So this patch can reduce redundant calling of psi_memstall_enter. And > > > > > make it easier to track refaulting file pages and anonymous pages > > > > > separately. > > > > > > > > I don't think this is an improvement. > > > > > > > > psi_memstall_enter() will check current->in_memstall once, detect the > > > > nested call, and bail. Your patch checks PageSwapBacked for every page > > > > being added. It's more branches for less robust code. > > > > > > We are also working for a new patch to classify different reasons cause > > > psi_memstall_enter(): reclaim, thrashing, compact, etc. This will help > > > user to tuning sysctl, for example, if user see high compact delay, he > > > may try do adjust THP sysctl to reduce the compact delay. > > > > > > To support that, we should distinguish what's the reason cause psi in > > > submit_io(), this patch does the job. > > > > Please submit these patches together then. On its own, this patch > > isn't desirable. > I think this patch has it's independent value, I try to make a better > explain. > > 1) This patch doesn't work it worse or even better > After this patch, swap workingset handle is simpler, file workingset > handle just has one more check, as below. > Before this patch handling swap workingset: > a) in swap_readpage() call psi_memstall_enter() -> > b) in __bio_add_page() test if should call bio_set_flag(), true -> > c) in __bio_add_page() call bio_set_flag() > d) in submit_bio() test if should call psi_memstall_enter(), true -> > e) call psi_memstall_enter, detect the nested call, and bail. > f) call bio_clear_flag if needed. > Before this patch handling file page workingset: > a) in __bio_add_page() test if should call bio_set_flag(), true -> > ... > b) call bio_clear_flag if needed. > After this patch handling swap workingset: > a) in swap_readpage() call psi_memstall_enter() -> > b) in __bio_add_page() test if should call bio_set_flag(), one more check, false and return. > c) in submit_bio() test if should call psi_memstall_enter(), false and return. > After this patch handling file pages workingset: > a) in __bio_add_page() test if should call bio_set_flag(), one more check, true -> > ... > b) call bio_clear_flag if needed. > > 2) This patch help tools like kprobe to trace different workingset > After this patch we know workingset in submit_io() is only cause by file pages. > > 3) This patch will help code evolution > Such as psi classify, getdelays supports counting file pages workingset submit. Looking forward to your review, thanks!
