From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752650AbcF2HMO (ORCPT ); Wed, 29 Jun 2016 03:12:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53536 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752607AbcF2HMM (ORCPT ); Wed, 29 Jun 2016 03:12:12 -0400 Date: Wed, 29 Jun 2016 09:12:07 +0200 From: Jiri Olsa To: Rasmus Villemoes Cc: Jiri Olsa , Steven Rostedt , lkml , Frederic Weisbecker Subject: Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask Message-ID: <20160629071207.GA14163@krava> References: <1467128075-10841-1-git-send-email-jolsa@kernel.org> <87r3bhgge8.fsf@rasmusvillemoes.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87r3bhgge8.fsf@rasmusvillemoes.dk> User-Agent: Mutt/1.6.1 (2016-04-27) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 29 Jun 2016 07:12:10 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 28, 2016 at 11:19:59PM +0200, Rasmus Villemoes wrote: SNIP > > - save_arg(void *); > > + if (spec.cpumask) { > > As I hinted in the other mail, I think it's better just to put the > fmt[1]=='b' here and not change struct printf_spec. > > > + /* > > + * Store entire cpumask directly to buffer > > + * instead of storing just a pointer. > > + */ > > + struct cpumask *mask = va_arg(args, void *); > > + > > + str = PTR_ALIGN(str, sizeof(u32)); > > + > > + if (str + sizeof(*mask) <= end) > > + cpumask_copy((struct cpumask *) str, mask); > > A cpumask is an array of longs. Why is u32-alignment enough for that? > cpumask_copy may end up compiling to a simple "*dst = *src", and even if > this is a memcpy(), the same 4-but-possibly-not-8 byte aligned > pointer is created below in bstr_printf which is then passed on to > pointer() and then bitmap_* which certainly expects an unsigned long*. hum, the binary buffer is copied after to trace buffer which has 32bit alignment I think, so bigger alignment breaks the data.. also the save_arg macro does 32bit alignment for 8 bytes anyway, I haven't checked deeply on this, was just hunting wrong cpumask, now with Steven's changes I'm ok to drop it ;-) thanks, jirka