Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE
To:     Peter Zijlstra <peterz@infradead.org>,
        Song Liu <songliubraving@fb.com>
References: <20171115172339.1791161-1-songliubraving@fb.com>
 <20171115172339.1791161-3-songliubraving@fb.com>
 <20171123100214.qm3s4h5b6i4r2iaa@hirez.programming.kicks-ass.net>
CC:     <rostedt@goodmis.org>, <mingo@redhat.com>, <davem@davemloft.net>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <daniel@iogearbox.net>, <kernel-team@fb.com>
From:   Alexei Starovoitov <ast@fb.com>
Message-ID: <adfbe341-7599-6819-27ef-4bc13cb65d6d@fb.com>
Date:   Thu, 23 Nov 2017 22:31:29 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20171123100214.qm3s4h5b6i4r2iaa@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Received-SPF: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Nov 2017 06:31:34.5453 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: a8837498-b81a-4c99-538c-08d53305026b
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR15MB0965
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-24_02:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/23/17 2:02 AM, Peter Zijlstra wrote:
> On Wed, Nov 15, 2017 at 09:23:33AM -0800, Song Liu wrote:
>
>> Note: We use type __u64 for pointer probe_desc instead of __aligned_u64.
>> The reason here is to avoid changing the size of struct perf_event_attr,
>> and breaking new-kernel-old-utility scenario. To avoid alignment problem
>> with the pointer, we will (in the following patches) copy probe_desc to
>> __aligned_u64 before using it as pointer.
>
> ISTR there are only relatively few architectures where __u64 and
> __aligned_u64 are not the same thing.
>
> The comment that goes with it seems to suggest i386 has short alignment
> for u64 but my compiler says differently:
>
>         printf("%d, %d\n", sizeof(unsigned long long), __alignof__(unsigned long long));
>
> $ gcc -m32 -o align align.c && ./align
> 8, 8

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include <stdio.h>

struct S {
   unsigned long long a;
} s;

struct U {
   unsigned long long a;
} u;

int main()
{
         printf("%d, %d\n", sizeof(unsigned long long),
                __alignof__(unsigned long long));
         printf("%d, %d\n", sizeof(s), __alignof__(s));
         printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4

so we have to use __aligned_u64 in uapi.

Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.
If you prefer we can do the same around 'config1', but it's not
any prettier.
We considered adding __aligned_u64 to the end of
'struct perf_event_attr', but it's a waste for most users, so reusing
the space of 'config' field like this seems the least evil.


From 1584851945917659441@xxx Thu Nov 23 10:23:16 +0000 2017
X-GM-THRID: 1584155725693670341
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread