Re: [RFC PATCH 68/86] treewide: mm: remove cond_resched()

[Yosry Ahmed]

On Tue, Nov 7, 2023 at 11:49 PM Vlastimil Babka <vbabka@xxxxxxx> wrote:
>
> On 11/8/23 02:28, Sergey Senozhatsky wrote:
> > On (23/11/07 15:08), Ankur Arora wrote:
> > [..]
> >> +++ b/mm/zsmalloc.c
> >> @@ -2029,7 +2029,6 @@ static unsigned long __zs_compact(struct zs_pool *pool,
> >>                      dst_zspage = NULL;
> >>
> >>                      spin_unlock(&pool->lock);
> >> -                    cond_resched();
> >>                      spin_lock(&pool->lock);
> >>              }
> >>      }
> >
> > I'd personally prefer to have a comment explaining why we do that
> > spin_unlock/spin_lock sequence, which may look confusing to people.
>
> Wonder if it would make sense to have a lock operation that does the
> unlock/lock as a self-documenting thing, and maybe could also be optimized
> to first check if there's a actually a need for it (because TIF_NEED_RESCHED
> or lock is contended).

+1, I was going to suggest this as well. It can be extended to other
locking types that disable preemption as well like RCU. Something like
spin_lock_relax() or something.

>
> > Maybe would make sense to put a nice comment in all similar cases.
> > For instance:
> >
> >       rcu_read_unlock();
> >  -    cond_resched();
> >       rcu_read_lock();
>
>