From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751392AbbLUHmO (ORCPT ); Mon, 21 Dec 2015 02:42:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:55571 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750881AbbLUHmM (ORCPT ); Mon, 21 Dec 2015 02:42:12 -0500 Subject: Re: [PATCH V3 4/4] scsi: storvsc: Tighten up the interrupt path To: KY Srinivasan , James Bottomley , "gregkh@linuxfoundation.org" , "linux-kernel@vger.kernel.org" , "devel@linuxdriverproject.org" , "ohering@suse.com" , "jbottomley@parallels.com" , "hch@infradead.org" , "linux-scsi@vger.kernel.org" , "apw@canonical.com" , "vkuznets@redhat.com" , "jasowang@redhat.com" , "martin.petersen@oracle.com" References: <1450038486-19211-1-git-send-email-kys@microsoft.com> <1450038512-19252-1-git-send-email-kys@microsoft.com> <1450038512-19252-4-git-send-email-kys@microsoft.com> <5673C92B.5070602@suse.de> <1450457288.2439.7.camel@HansenPartnership.com> From: Hannes Reinecke Message-ID: <5677AD50.4030406@suse.de> Date: Mon, 21 Dec 2015 08:42:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/19/2015 03:28 AM, KY Srinivasan wrote: > [ .. ] >> >> Could you? You're making what you describe as an optimisation but >> there are two reasons why this might not be so. The first is that the >> compiler is entitled to inline static functions. If it did, likely it >> picked up the optmisation anyway as Hannes suggested. However, the >> other reason this might not be an optimisation (assuming the compiler >> doesn't inline the function) is you're passing an argument which can be >> offset computed. On all architectures, you have a fixed number of >> registers for passing function arguments, then we have to use the >> stack. Using the stack comes in far more expensive than computing an >> offset to an existing pointer. Even if you're still in registers, the >> offset now has to be computed and stored and the compiler loses track >> of the relation. >> >> The bottom line is that adding an extra argument for a value which can >> be offset computed is rarely a win. > > James, > When I did this, I was mostly concerned about the cost of reestablishing state that was > already known. So, even with the function being in-lined, I felt the cost of reestablishing > state that was already known is unnecessary. In this particular case, I did not change the > number of arguments that were being passed; I just changed the type of one of them - > instead of passing struct hv_device *, I am now passing struct storvsc_device *. In the > current code, we are using struct hv_device * to establish a pointer to struct storvsc_device * > via the function get_in_stor_device(). This pattern currently exists in the call chain from the > interrupt handler - storvsc_on_channel_callback(). > > While the compiler is smart enough to inline both get_in_stor_device() as well as many of the static > functions in the call chain from storvsc_on_channel_callback(), looking at the assembled code, > the compiler is repeatedly inlining the call to get_in_stor_device() and this clearly is less than optimal. > Which means you actually checked the compiler output, and it made a difference. That's all I wanted to know, as it's not immediately clear from the patch. So: Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)