From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755695Ab0IQRYn (ORCPT ); Fri, 17 Sep 2010 13:24:43 -0400 Received: from sj-iport-4.cisco.com ([171.68.10.86]:8985 "EHLO sj-iport-4.cisco.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754374Ab0IQRYl (ORCPT ); Fri, 17 Sep 2010 13:24:41 -0400 Authentication-Results: sj-iport-4.cisco.com; dkim=neutral (message not signed) header.i=none X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AsMFAA9Bk0yrR7H+/2dsb2JhbACUNo1vcagZnDmCc4JNBIRMhWeEdw X-IronPort-AV: E=Sophos;i="4.56,383,1280707200"; d="scan'208";a="188266401" Message-ID: <4C93A459.1000009@cisco.com> Date: Fri, 17 Sep 2010 10:24:41 -0700 From: Joe Eykholt User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.9) Gecko/20100825 Lightning/1.0b2 Thunderbird/3.1.3 MIME-Version: 1.0 To: linux-iscsi-target-dev@googlegroups.com CC: "Nicholas A. Bellinger" , James Bottomley , Andi Kleen , linux-scsi , linux-kernel , Vasu Dev , Tim Chen , Matthew Wilcox , Mike Christie , James Smart , Andrew Vasquez , FUJITA Tomonori , Hannes Reinecke , Christoph Hellwig Subject: Re: [PATCH 1/8] scsi: Drop struct Scsi_Host->host_lock around SHT->queuecommand() References: <1284676529-10756-1-git-send-email-nab@linux-iscsi.org> <1284691571.26423.50.camel@mulgrave.site> <20100917072022.GB2644@gargoyle.ger.corp.intel.com> <1284725592.26423.60.camel@mulgrave.site> <4C9379AA.4000103@linux.intel.com> <1284735438.26423.81.camel@mulgrave.site> <1284741478.13344.154.camel@haakon2.linux-iscsi.org> In-Reply-To: <1284741478.13344.154.camel@haakon2.linux-iscsi.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9/17/10 9:37 AM, Nicholas A. Bellinger wrote: > On Fri, 2010-09-17 at 10:57 -0400, James Bottomley wrote: >> On Fri, 2010-09-17 at 16:22 +0200, Andi Kleen wrote: >>>> I don't disagree with the idea of removing it, especially as it has so >>>> few users, but replacing the host lock with an atomic here would still >>>> vastly reduce the contention, which is the initial complaint. The >>> >>> Actually the complaint is the overhead of the spin lock. This can be >>> either caused >>> by contention or by cache line bounce time. >> >> The original complaint was contention. My desire is to reduce the >> locked path coverage, so I saw an opportunity. >> >> What I was actually thinking of for the atomic is that we'd let it range >> [1..INT_MAX] so a zero was an indicator of no use of this. Then the >> actual code could become >> >> if (atomic_read(x)) { >> do { >> y = atomic_add_return(1, x); >> } while (y == 0); >> } A tiny trick I like to use is to start a serial number at 1 and increment by 2 so its always odd and then never wraps to 0. That eliminates the check for 0 (and the curly brackets). > The conversion of struct scsi_cmnd->serial_number to atomic_t and the > above code for scsi_cmd_get_serial() sounds perfectly reasonable to me. > > I will take a look at this conversion and respin a complete set of > patches for review a bit later today. > > Thanks! > > --nab > >> >> So "fast" cards not using the serial number set a zero there (we'd >> default initialise to one), the line is shared so no bouncing (because >> it's never updated). This should satisfy everyone. >> >>>> contention occurs because the host lock is so widely used for other >>>> things. The way to reduce that contention is firstly to reduce the >>>> length of code covered by the lock and also reduce the actual number of >>>> places where the lock is taken. Compared with host lock's current vast >>>> footprint, and atomic here is tiny. >>> >>> That assumes that it's contention that is the problem and not simply >>> bounce time. >> >> That's what the patch and data that started this whole thread showed, >> yes ... but I think actual bounce in the spinlock is also a problem ... >> we just don't have data to show it. >> >> James >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >