Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1712683

Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature

From Byungchul Park <byungchul.park@lge.com>
Newsgroups linux.kernel
Subject Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature
Date 2017-08-16 07:10 +0200
Message-ID <ueYKB-2PV-1@gated-at.bofh.it> (permalink)
References <ubKut-59V-3@gated-at.bofh.it> <ueFoB-7kE-1@gated-at.bofh.it> <ueUdX-8jK-9@gated-at.bofh.it> <ueXOx-2cs-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, Aug 16, 2017 at 12:05:31PM +0800, Boqun Feng wrote:
> > I see...
> > 
> > Worker A : acquired of wfc.work -> wait for cpu_hotplug_lock to be released
> > Task   B : acquired of cpu_hotplug_lock -> wait for lock#3 to be released
> > Task   C : acquired of lock#3 -> wait for completion of barr->done
> 
> >From the stack trace below, this barr->done is for flush_work() in
> lru_add_drain_all_cpuslocked(), i.e. for work "per_cpu(lru_add_drain_work)"
> 
> > Worker D : wait for wfc.work to be released -> will complete barr->done
> 
> and this barr->done is for work "wfc.work".
> 
> So those two barr->done could not be the same instance, IIUC. Therefore
> the deadlock case is not possible.
> 
> The problem here is all barr->done instances are initialized at
> insert_wq_barrier() and they belongs to the same lock class, to fix

I'm not sure this caused the lockdep warning but, if they belongs to the
same class even though they couldn't be the same instance as you said, I
also think that is another problem and should be fixed.

> this, we need to differ barr->done with different lock classes based on
> the corresponding works.
> 
> How about the this(only compilation test):
> 
> ----------------->8
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index e86733a8b344..d14067942088 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2431,6 +2431,27 @@ struct wq_barrier {
>  	struct task_struct	*task;	/* purely informational */
>  };
>  
> +#ifdef CONFIG_LOCKDEP_COMPLETE
> +# define INIT_WQ_BARRIER_ONSTACK(barr, func, target)				\
> +do {										\
> +	INIT_WORK_ONSTACK(&(barr)->work, func);					\
> +	__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&(barr)->work));	\
> +	lockdep_init_map_crosslock((struct lockdep_map *)&(barr)->done.map,	\
> +				   "(complete)" #barr,				\
> +				   (target)->lockdep_map.key, 1); 		\
> +	__init_completion(&barr->done);						\
> +	barr->task = current;							\
> +} while (0)
> +#else
> +# define INIT_WQ_BARRIER_ONSTACK(barr, func, target)				\
> +do {										\
> +	INIT_WORK_ONSTACK(&(barr)->work, func);					\
> +	__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&(barr)->work));	\
> +	init_completion(&barr->done);						\
> +	barr->task = current;							\
> +} while (0)
> +#endif
> +
>  static void wq_barrier_func(struct work_struct *work)
>  {
>  	struct wq_barrier *barr = container_of(work, struct wq_barrier, work);
> @@ -2474,10 +2495,7 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
>  	 * checks and call back into the fixup functions where we
>  	 * might deadlock.
>  	 */
> -	INIT_WORK_ONSTACK(&barr->work, wq_barrier_func);
> -	__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&barr->work));
> -	init_completion(&barr->done);
> -	barr->task = current;
> +	INIT_WQ_BARRIER_ONSTACK(barr, wq_barrier_func, target);
>  
>  	/*
>  	 * If @target is currently being executed, schedule the

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Ingo Molnar <mingo@kernel.org> - 2017-08-15 10:30 +0200
  Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 02:20 +0200
    Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Boqun Feng <boqun.feng@gmail.com> - 2017-08-16 06:10 +0200
      Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 06:40 +0200
        Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Boqun Feng <boqun.feng@gmail.com> - 2017-08-16 07:50 +0200
          Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 08:40 +0200
      Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 07:10 +0200
        Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Boqun Feng <boqun.feng@gmail.com> - 2017-08-16 08:00 +0200
          Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 09:20 +0200
            Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 10:10 +0200
              Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-08-16 11:50 +0200
      Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Ingo Molnar <mingo@kernel.org> - 2017-08-17 09:50 +0200
        Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Boqun Feng <boqun.feng@gmail.com> - 2017-08-17 10:10 +0200
          Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Ingo Molnar <mingo@kernel.org> - 2017-08-17 10:20 +0200
            Re: [PATCH v8 00/14] lockdep: Implement crossrelease feature Boqun Feng <boqun.feng@gmail.com> - 2017-08-17 10:40 +0200

csiph-web