Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752345AbbDQVBF (ORCPT ); Fri, 17 Apr 2015 17:01:05 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:50591 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750865AbbDQVBC (ORCPT ); Fri, 17 Apr 2015 17:01:02 -0400 Date: Fri, 17 Apr 2015 14:00:51 -0700 From: Shaohua Li To: Jeff Moyer CC: , Subject: Re: [PATCH] blk: clean up plug Message-ID: <20150417210050.GA819331@devbig257.prn2.facebook.com> References: <01bb5a7d9d50d0dbbeb8eadc11dc110ee999c2b5.1429205153.git.shli@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-12-10) X-Originating-IP: [192.168.52.13] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.13.68,1.0.33,0.0.0000 definitions=2015-04-17_07:2015-04-17,2015-04-17,1970-01-01 signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2761 Lines: 59 On Fri, Apr 17, 2015 at 04:54:40PM -0400, Jeff Moyer wrote: > Shaohua Li writes: > > > Current code looks like inner plug gets flushed with a > > blk_finish_plug(). Actually it's a nop. All requests/callbacks are added > > to current->plug, while only outmost plug is assigned to current->plug. > > So inner plug always has empty request/callback list, which makes > > blk_flush_plug_list() a nop. This tries to make the code more clear. > > > > Signed-off-by: Shaohua Li > > Hi, Shaohua, > > I agree that this looks like a clean-up with no behavioral change, and > it looks good to me. However,it does make me scratch my head about the > numbers I was seeing. Here's the table from that other email thread[1]: > > device| vanilla | patch1 | dio-noplug | noflush-nested > ------+------------+----------------+---------------+----------------- > rssda | 701,684 | 1,168,527 | 1,342,177 | 1,297,612 > | 100% | +66% | +91% | +85% > vdb0 | 358,264 | 902,913 | 906,850 | 922,327 > | 100% | +152% | +153% | +157% > > Patch1 refers to the first patch in this series, which fixes the merge > logic for single-queue blk-mq devices. Each column after that includes > that first patch. In dio-noplug, I removed the blk_plug from the > direct-io code path (so there is no nesting at all). This is a control, > since it is what I expect the outcome of the noflush-nested column to > actually be. Then, the noflush-nested column leaves the blk_plug in > place in the dio code, but includes the patch that prevents nested > blk_plug's from being flushed. All numbers are the average of 5 runs. > With the exception of the vanilla run on rssda (the first run was > faster, causing the average to go up), the standard deviation is very > small. > > For the dio-noplug column, if the inner plug really was a noop, then why > would we see any change in performance? Like I said, I agree with your > reading of the code and the patch. Color me confused. I'll poke at it > more next week. For now, I think your patch is fine. > > Reviewed-by: Jeff Moyer Thanks! I don't know why either the your second makes change. > Also, Jens or Shaohua or anyone, please review my blk-mq plug fix (patch > 1/2 of aforementioned thread). ;) You are not alone :), I posted 2 times too http://marc.info/?l=linux-kernel&m=142627559617005&w=2 Thanks, Shaohua -- 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/