Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 3:34 PM, Al Viro wrote: > On Sun, Apr 15, 2018 at 10:54:55PM +0100, Al Viro wrote: >> On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: >> >> > BTW, the current placement of cond_resched() looks bogus; suppose we >> > have collected a lot of

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 3:34 PM, Al Viro wrote: > On Sun, Apr 15, 2018 at 10:54:55PM +0100, Al Viro wrote: >> On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: >> >> > BTW, the current placement of cond_resched() looks bogus; suppose we >> > have collected a lot of victims and ran into

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 10:54:55PM +0100, Al Viro wrote: > On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: > > > BTW, the current placement of cond_resched() looks bogus; suppose we > > have collected a lot of victims and ran into need_resched(). We leave > > d_walk() and call

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 10:54:55PM +0100, Al Viro wrote: > On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: > > > BTW, the current placement of cond_resched() looks bogus; suppose we > > have collected a lot of victims and ran into need_resched(). We leave > > d_walk() and call

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: > BTW, the current placement of cond_resched() looks bogus; suppose we > have collected a lot of victims and ran into need_resched(). We leave > d_walk() and call shrink_dentry_list(). At that point there's a lot > of stuff on our shrink

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 09:40:54PM +0100, Al Viro wrote: > BTW, the current placement of cond_resched() looks bogus; suppose we > have collected a lot of victims and ran into need_resched(). We leave > d_walk() and call shrink_dentry_list(). At that point there's a lot > of stuff on our shrink

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 11:34:17AM -0700, Linus Torvalds wrote: > No, it's obviously not type-safe, but at least it's _legible_, and is > a pattern, while that "let's randomly just do a cast in the middle of > the code" is just nasty. Sure, no problem... I really wish there was a way to say

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 11:34:17AM -0700, Linus Torvalds wrote: > No, it's obviously not type-safe, but at least it's _legible_, and is > a pattern, while that "let's randomly just do a cast in the middle of > the code" is just nasty. Sure, no problem... I really wish there was a way to say

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 5:51 PM, Al Viro wrote: >> if (!list_empty()) That was obviously meant to be just if (data.select.found) I had just cut-and-pasted a bit too much. > You would have to do the same in check_and_drop() as well, >

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 5:51 PM, Al Viro wrote: >> if (!list_empty()) That was obviously meant to be just if (data.select.found) I had just cut-and-pasted a bit too much. > You would have to do the same in check_and_drop() as well, > and that brings back

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 03:39:44AM +0100, Al Viro wrote: > I really wonder if we should just do the following in > d_invalidate(): > * grab ->d_lock on victim, check if it's unhashed, > unlock and bugger off if it is. Otherwise, unhash and unlock. > >From that point on any d_set_mounted()

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 03:39:44AM +0100, Al Viro wrote: > I really wonder if we should just do the following in > d_invalidate(): > * grab ->d_lock on victim, check if it's unhashed, > unlock and bugger off if it is. Otherwise, unhash and unlock. > >From that point on any d_set_mounted()

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 01:51:07AM +0100, Al Viro wrote: > On Sat, Apr 14, 2018 at 02:47:21PM -0700, Linus Torvalds wrote: > > On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > > > > > That breaks d_invalidate(), unfortunately. Look at the termination > > > conditions

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sun, Apr 15, 2018 at 01:51:07AM +0100, Al Viro wrote: > On Sat, Apr 14, 2018 at 02:47:21PM -0700, Linus Torvalds wrote: > > On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > > > > > That breaks d_invalidate(), unfortunately. Look at the termination > > > conditions in the loop there... > >

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 02:47:21PM -0700, Linus Torvalds wrote: > On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > > > That breaks d_invalidate(), unfortunately. Look at the termination > > conditions in the loop there... > > Ugh. I was going to say "but that doesn't

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 02:47:21PM -0700, Linus Torvalds wrote: > On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > > > That breaks d_invalidate(), unfortunately. Look at the termination > > conditions in the loop there... > > Ugh. I was going to say "but that doesn't even use

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > That breaks d_invalidate(), unfortunately. Look at the termination > conditions in the loop there... Ugh. I was going to say "but that doesn't even use select_collect()", but yeah, detach_and_collect() calls it. It

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 1:58 PM, Al Viro wrote: > > That breaks d_invalidate(), unfortunately. Look at the termination > conditions in the loop there... Ugh. I was going to say "but that doesn't even use select_collect()", but yeah, detach_and_collect() calls it. It would be easy enough to

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 09:36:23AM -0700, Linus Torvalds wrote: > But it does *not* make sense for the case where we've hit a dentry > that is already on the shrink list. Sure, we'll continue to gather all > the other dentries, but if there is concurrent shrinking, shouldn't we > give up the CPU

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 09:36:23AM -0700, Linus Torvalds wrote: > But it does *not* make sense for the case where we've hit a dentry > that is already on the shrink list. Sure, we'll continue to gather all > the other dentries, but if there is concurrent shrinking, shouldn't we > give up the CPU

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 1:02 AM, Al Viro wrote: > > "Bail out" is definitely a bad idea, "sleep"... what on? Especially > since there might be several evictions we are overlapping with... Well, one thing that should be looked at is the return condition from

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 1:02 AM, Al Viro wrote: > > "Bail out" is definitely a bad idea, "sleep"... what on? Especially > since there might be several evictions we are overlapping with... Well, one thing that should be looked at is the return condition from select_collect() that

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 10:00:29AM +0300, Nikolay Borisov wrote: > > > On 14.04.2018 00:14, Andrew Morton wrote: > > On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov > > wrote: > > > >> shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > >> In

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Sat, Apr 14, 2018 at 10:00:29AM +0300, Nikolay Borisov wrote: > > > On 14.04.2018 00:14, Andrew Morton wrote: > > On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov > > wrote: > > > >> shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > >> In this case we may have

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On 14.04.2018 00:14, Andrew Morton wrote: > On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov > wrote: > >> shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. >> In this case we may have 0 dentries to dispose, so we will never >> schedule out while

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On 14.04.2018 00:14, Andrew Morton wrote: > On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov > wrote: > >> shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. >> In this case we may have 0 dentries to dispose, so we will never >> schedule out while waiting for the

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Fri, 13 Apr 2018, Khazhismel Kumykov wrote: > shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > In this case we may have 0 dentries to dispose, so we will never > schedule out while waiting for the parallel shrink_dentry_list to > complete. > > Tested that this fixes

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Fri, 13 Apr 2018, Khazhismel Kumykov wrote: > shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > In this case we may have 0 dentries to dispose, so we will never > schedule out while waiting for the parallel shrink_dentry_list to > complete. > > Tested that this fixes

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov wrote: > shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > In this case we may have 0 dentries to dispose, so we will never > schedule out while waiting for the parallel shrink_dentry_list to >

Re: [PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

On Fri, 13 Apr 2018 13:28:23 -0700 Khazhismel Kumykov wrote: > shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. > In this case we may have 0 dentries to dispose, so we will never > schedule out while waiting for the parallel shrink_dentry_list to > complete. > > Tested

[PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. In this case we may have 0 dentries to dispose, so we will never schedule out while waiting for the parallel shrink_dentry_list to complete. Tested that this fixes syzbot reports of stalls in shrink_dcache_parent() Fixes:

[PATCH] fs/dcache.c: re-add cond_resched() in shrink_dcache_parent()

shrink_dcache_parent may spin waiting for a parallel shrink_dentry_list. In this case we may have 0 dentries to dispose, so we will never schedule out while waiting for the parallel shrink_dentry_list to complete. Tested that this fixes syzbot reports of stalls in shrink_dcache_parent() Fixes: