From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756487AbbI2PBl (ORCPT ); Tue, 29 Sep 2015 11:01:41 -0400 Received: from mail-ig0-f173.google.com ([209.85.213.173]:34565 "EHLO mail-ig0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754140AbbI2PBe (ORCPT ); Tue, 29 Sep 2015 11:01:34 -0400 Subject: Re: [PATCH v4 6/7] blk-mq: fix freeze queue race To: Tejun Heo , Akinobu Mita References: <1443287365-4244-1-git-send-email-akinobu.mita@gmail.com> <1443287365-4244-7-git-send-email-akinobu.mita@gmail.com> <20150926173256.GA3572@htj.duckdns.org> <20150928144839.GA2589@mtj.duckdns.org> Cc: LKML , Ming Lei , Christoph Hellwig From: Jens Axboe Message-ID: <560AA7CB.2070107@kernel.dk> Date: Tue, 29 Sep 2015 09:01:31 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150928144839.GA2589@mtj.duckdns.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/28/2015 08:48 AM, Tejun Heo wrote: > Hello, > > On Sun, Sep 27, 2015 at 10:06:05PM +0900, Akinobu Mita wrote: >>>> void blk_mq_finish_init(struct request_queue *q) >>>> { >>>> + mutex_lock(&q->mq_freeze_lock); >>>> percpu_ref_switch_to_percpu(&q->mq_usage_counter); >>>> + mutex_unlock(&q->mq_freeze_lock); >>> >>> This looks weird to me. What can it race against at this point? >> >> The possible scenario is described in commit log (1. ~ 7.). In summary, >> blk_mq_finish_init() and blk_mq_freeze_queue_start() can be executed >> at the same time, so this is required to serialize the execution of >> percpu_ref_switch_to_percpu() by blk_mq_finish_init() and >> percpu_ref_kill() by blk_mq_freeze_queue_start(). > > Ah, you're right. I was thinking that percpu_ref_switch_to_percpu() > being called after blk_mq_freeze_queue_start() would be buggy and thus > the above can't be enough but that is safe as long as the calls are > properly synchronized. Hmmm... maybe we should add synchronization to > those operations from percpu_ref side. I think that would be very useful, it seems sort of half-assed if the caller side has to provide serialization for that. -- Jens Axboe