From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751301Ab1JIHQl (ORCPT ); Sun, 9 Oct 2011 03:16:41 -0400 Received: from mail-qw0-f46.google.com ([209.85.216.46]:51152 "EHLO mail-qw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750893Ab1JIHQk convert rfc822-to-8bit (ORCPT ); Sun, 9 Oct 2011 03:16:40 -0400 MIME-Version: 1.0 In-Reply-To: References: <1317397918.27140.15.camel@localhost> <1317729761.25998.4.camel@localhost> <20111008161421.GA5743@redhat.com> Date: Sun, 9 Oct 2011 15:16:39 +0800 X-Google-Sender-Auth: 11xasitlzVUj1BdCfpD9c6N2oY8 Message-ID: Subject: Re: Block regression since 3.1-rc3 From: Shaohua Li To: Mike Snitzer Cc: Jeff Moyer , Jens Axboe , Tejun Heo , device-mapper development , Christophe Saout , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2011/10/9 Shaohua Li : > 2011/10/9 Mike Snitzer : >> On Sat, Oct 08 2011 at  7:02am -0400, >> Shaohua Li wrote: >> >>> Looks the dm request based flush logic is broken. >>> >>> saved_make_request_fn >>>   __make_request >>>     blk_insert_flush >>> but blk_insert_flush doesn't put the original request to list, instead, the >>> q->flush_rq is in list. >>> then >>> dm_request_fn >>>   blk_peek_request >>>     dm_prep_fn >>>       clone_rq >>>   map_request >>>     blk_insert_cloned_request >>> so q->flush_rq is cloned, and get dispatched. but we can't clone q->flush_rq >>> and use it to do flush. map_request even could assign a different blockdev to >>> the cloned request. >> >> You haven't explained why cloning q->flush_rq is broken.  What is the >> problem with map_request changing the blockdev?  For the purposes of >> request-based DM the flush machinery has already managed the processing >> of the flush at the higher level request_queue. > hmm, looks I overlook the issue. cloned flush_rq has some problems but can > be fixed. > 1. it doesn't set requet->bio, request->bio_tail > 2. REQ_CLONE_MASK should set REQ_FLUSH_SEQ oh, don't need set REQ_FLUSH_SEQ, the low level queue will set it anyway. sorry for the noise. Jeff's patch looks ok then. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shaohua Li Subject: Re: Block regression since 3.1-rc3 Date: Sun, 9 Oct 2011 15:16:39 +0800 Message-ID: References: <1317397918.27140.15.camel@localhost> <1317729761.25998.4.camel@localhost> <20111008161421.GA5743@redhat.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Mike Snitzer Cc: Jens Axboe , Christophe Saout , device-mapper development , linux-kernel@vger.kernel.org, Jeff Moyer , Tejun Heo List-Id: dm-devel.ids 2011/10/9 Shaohua Li : > 2011/10/9 Mike Snitzer : >> On Sat, Oct 08 2011 at =A07:02am -0400, >> Shaohua Li wrote: >> >>> Looks the dm request based flush logic is broken. >>> >>> saved_make_request_fn >>> =A0 __make_request >>> =A0 =A0 blk_insert_flush >>> but blk_insert_flush doesn't put the original request to list, instead,= the >>> q->flush_rq is in list. >>> then >>> dm_request_fn >>> =A0 blk_peek_request >>> =A0 =A0 dm_prep_fn >>> =A0 =A0 =A0 clone_rq >>> =A0 map_request >>> =A0 =A0 blk_insert_cloned_request >>> so q->flush_rq is cloned, and get dispatched. but we can't clone q->flu= sh_rq >>> and use it to do flush. map_request even could assign a different block= dev to >>> the cloned request. >> >> You haven't explained why cloning q->flush_rq is broken. =A0What is the >> problem with map_request changing the blockdev? =A0For the purposes of >> request-based DM the flush machinery has already managed the processing >> of the flush at the higher level request_queue. > hmm, looks I overlook the issue. cloned flush_rq has some problems but can > be fixed. > 1. it doesn't set requet->bio, request->bio_tail > 2. REQ_CLONE_MASK should set REQ_FLUSH_SEQ oh, don't need set REQ_FLUSH_SEQ, the low level queue will set it anyway. sorry for the noise. Jeff's patch looks ok then.