Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751851AbbKDC4s (ORCPT ); Tue, 3 Nov 2015 21:56:48 -0500 Received: from mga09.intel.com ([134.134.136.24]:61593 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750891AbbKDC4r (ORCPT ); Tue, 3 Nov 2015 21:56:47 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,241,1444719600"; d="scan'208";a="593603793" Message-ID: <1446605804.17120.179.camel@tzanussi-mobl.amr.corp.intel.com> Subject: Re: [PATCH v11 08/28] tracing: Add lock-free tracing_map From: Tom Zanussi To: Namhyung Kim Cc: rostedt@goodmis.org, daniel.wagner@bmw-carit.de, masami.hiramatsu.pt@hitachi.com, josh@joshtriplett.org, andi@firstfloor.org, mathieu.desnoyers@efficios.com, peterz@infradead.org, linux-kernel@vger.kernel.org Date: Tue, 03 Nov 2015 20:56:44 -0600 In-Reply-To: <20151104022614.GB19283@sejong> References: <20151029083114.GA2617@sejong> <1446143743.4661.58.camel@tzanussi-mobl.amr.corp.intel.com> <20151102070805.GB17941@sejong> <1446601672.17120.164.camel@tzanussi-mobl.amr.corp.intel.com> <20151104022614.GB19283@sejong> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3559 Lines: 107 Hi Namhyung, On Wed, 2015-11-04 at 11:26 +0900, Namhyung Kim wrote: > Hi Tom, > > On Tue, Nov 03, 2015 at 07:47:52PM -0600, Tom Zanussi wrote: > > Hi Namhyung, > > > > On Mon, 2015-11-02 at 16:08 +0900, Namhyung Kim wrote: > > > I thought it'd be better if users can see which one is the real drop > > > or not. IOW if drop count is much smaller than the normal event > > > count, [s]he might want to ignore the occasional drops. Otherwise, > > > [s]he should restart with a bigger table. This requires accurate > > > counts of events and drops though. > > > > > > > OK, how about the below - it basically moves the drops set/test/inc into > > tracing_map_insert(), as well as a total hits count. So those values > > will be available for users to use in deciding whether to use the data > > or restart with a bigger table, and the loop is bailed out of only if no > > matching keys are found and there are drops, so callers can continue > > updating existing entries. > > But if a key didn't get a desired index, it'd still fail to update.. > > > > > > Users who want the original behavior still get the NULL return and can > > stop calling tracing_map_insert() as before: > > > > struct tracing_map_elt *tracing_map_insert(struct tracing_map *map, void *key) > > { > > u32 idx, key_hash, test_key; > > struct tracing_map_entry *entry; > > > > key_hash = jhash(key, map->key_size, 0); > > if (key_hash == 0) > > key_hash = 1; > > idx = key_hash >> (32 - (map->map_bits + 1)); > > > > while (1) { > > idx &= (map->map_size - 1); > > entry = TRACING_MAP_ENTRY(map->map, idx); > > test_key = entry->key; > > > > if (test_key && test_key == key_hash && entry->val && > > keys_match(key, entry->val->key, map->key_size)) { > > atomic64_inc(&map->hits); > > return entry->val; > > } > > > > if (atomic64_read(&map->drops)) { > > atomic64_inc(&map->drops); > > break; > > } > > IMHO it should be removed. > > > > > if (!test_key && !cmpxchg(&entry->key, 0, key_hash)) { > > struct tracing_map_elt *elt; > > > > elt = get_free_elt(map); > > if (!elt) { > > atomic64_inc(&map->drops); > > And reset entry->key here.. > > > break; > > } > > memcpy(elt->key, key, map->key_size); > > entry->val = elt; > > > > atomic64_inc(&map->hits); > > return entry->val; > > } > > idx++; > > } > > > > return NULL; > > } > > > Then tracing_map_lookup() can be implemented like *_insert but bail out > from the loop if test_key is 0. The caller might do like this: > > if (!atomic64_read(&map->drops)) > elt = tracing_map_insert(...); > else > elt = tracing_map_lookup(...); > Yeah, I think that should work - let me try that in the next version... Thanks, Tom > Thanks, > Namhyung -- 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/