Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759215AbZDWHrY (ORCPT ); Thu, 23 Apr 2009 03:47:24 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756956AbZDWHcb (ORCPT ); Thu, 23 Apr 2009 03:32:31 -0400 Received: from sous-sol.org ([216.99.217.87]:50753 "EHLO x200.localdomain" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756934AbZDWHca (ORCPT ); Thu, 23 Apr 2009 03:32:30 -0400 Message-Id: <20090423072720.353676196@sous-sol.org> User-Agent: quilt/0.47-1 Date: Thu, 23 Apr 2009 00:21:20 -0700 From: Chris Wright To: linux-kernel@vger.kernel.org, stable@kernel.org, jejb@kernel.org Cc: Justin Forbes , Zwane Mwaikambo , "Theodore Ts'o" , Randy Dunlap , Dave Jones , Chuck Wolber , Chris Wedgwood , Michael Krufky , Chuck Ebbert , Domenico Andreoli , Willy Tarreau , Rodrigo Rubira Branco , Jake Edge , Eugene Teo , torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Mikulas Patocka , Alasdair G Kergon Subject: [patch 060/100] dm kcopyd: fix callback race References: <20090423072020.428683652@sous-sol.org> Content-Disposition: inline; filename=dm-kcopyd-fix-callback-race.patch Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2937 Lines: 81 -stable review patch. If anyone has any objections, please let us know. --------------------- From: Mikulas Patocka upstream commit: 340cd44451fb0bfa542365e6b4b565bbd44836e2 If the thread calling dm_kcopyd_copy is delayed due to scheduling inside split_job/segment_complete and the subjobs complete before the loop in split_job completes, the kcopyd callback could be invoked from the thread that called dm_kcopyd_copy instead of the kcopyd workqueue. dm_kcopyd_copy -> split_job -> segment_complete -> job->fn() Snapshots depend on the fact that callbacks are called from the singlethreaded kcopyd workqueue and expect that there is no racing between individual callbacks. The racing between callbacks can lead to corruption of exception store and it can also mean that exception store callbacks are called twice for the same exception - a likely reason for crashes reported inside pending_complete() / remove_exception(). This patch fixes two problems: 1. job->fn being called from the thread that submitted the job (see above). - Fix: hand over the completion callback to the kcopyd thread. 2. job->fn(read_err, write_err, job->context); in segment_complete reports the error of the last subjob, not the union of all errors. - Fix: pass job->write_err to the callback to report all error bits (it is done already in run_complete_job) Cc: stable@kernel.org Signed-off-by: Mikulas Patocka Signed-off-by: Alasdair G Kergon Signed-off-by: Chris Wright --- drivers/md/dm-kcopyd.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) --- a/drivers/md/dm-kcopyd.c +++ b/drivers/md/dm-kcopyd.c @@ -511,13 +511,16 @@ static void segment_complete(int read_er } else if (atomic_dec_and_test(&job->sub_jobs)) { /* - * To avoid a race we must keep the job around - * until after the notify function has completed. - * Otherwise the client may try and stop the job - * after we've completed. + * Queue the completion callback to the kcopyd thread. + * + * Some callers assume that all the completions are called + * from a single thread and don't race with each other. + * + * We must not call the callback directly here because this + * code may not be executing in the thread. */ - job->fn(read_err, write_err, job->context); - mempool_free(job, job->kc->job_pool); + push(&kc->complete_jobs, job); + wake(kc); } } @@ -530,6 +533,8 @@ static void split_job(struct kcopyd_job { int i; + atomic_inc(&job->kc->nr_jobs); + atomic_set(&job->sub_jobs, SPLIT_COUNT); for (i = 0; i < SPLIT_COUNT; i++) segment_complete(0, 0u, job); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/