From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754893AbcBBNFI (ORCPT ); Tue, 2 Feb 2016 08:05:08 -0500 Received: from mail-yk0-f196.google.com ([209.85.160.196]:34060 "EHLO mail-yk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754317AbcBBNFF (ORCPT ); Tue, 2 Feb 2016 08:05:05 -0500 Date: Tue, 2 Feb 2016 11:04:59 -0200 From: Gustavo Padovan To: Maarten Lankhorst , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Gustavo Padovan Subject: Re: [PATCH 06/10] staging/android: turn fence_info into a __64 pointer Message-ID: <20160202130459.GA29160@joana> Mail-Followup-To: Gustavo Padovan , Maarten Lankhorst , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Gustavo Padovan References: <1454102426-20637-1-git-send-email-gustavo@padovan.org> <1454102426-20637-7-git-send-email-gustavo@padovan.org> <56AF1A43.5010900@linux.intel.com> <20160201180008.GB3207@joana> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160201180008.GB3207@joana> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-02-01 Gustavo Padovan : > Hi Maarten, > > 2016-02-01 Maarten Lankhorst : > > > Op 29-01-16 om 22:20 schreef Gustavo Padovan: > > > From: Gustavo Padovan > > > > > > Making fence_info a pointer enables us to extend the struct in the future > > > without breaking the ABI. > > > > > > Signed-off-by: Gustavo Padovan > > > --- > > > drivers/staging/android/sync.c | 2 +- > > > drivers/staging/android/uapi/sync.h | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c > > > index f7530f0..51d4f47 100644 > > > --- a/drivers/staging/android/sync.c > > > +++ b/drivers/staging/android/sync.c > > > @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, > > > if (info->status >= 0) > > > info->status = !info->status; > > > > > > - len = sizeof(struct sync_file_info); > > > + len = sizeof(struct sync_file_info) - sizeof(__u64 *); > > > > > > for (i = 0; i < sync_file->num_fences; ++i) { > > > struct fence *fence = sync_file->cbs[i].fence; > > > diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h > > > index ed281fc..9f07aa7 100644 > > > --- a/drivers/staging/android/uapi/sync.h > > > +++ b/drivers/staging/android/uapi/sync.h > > > @@ -54,7 +54,7 @@ struct sync_file_info { > > > char name[32]; > > > __s32 status; > > > > > > - __u8 fence_info[0]; > > > + __u64 *fence_info; > > > }; > > > > > Pointers are awful, it should be a __u64 since it's a pointer type. Userspace should cast it to a uintptr_t in userspace. > > Oh, I made a mistake. I'll fix this. > > > > This structure also won't work on 64-bits systems, there may be a hole between fence_info and status (or num_fences in next patch). > > > > It's probably best to move it to the top and ensure the struct is 64-bits aligned. > > That is not possible because we are not allocating only 64bits there but > a array of struct fence_info, so it needs to be the last one. Maybe we > can add some sort of padding? Actually it is 64bits aligned: struct sync_file_info { char name[32]; __s32 status; __u32 flags; __u32 num_fences; __u32 len; __u64 fence_info; }; So nothing to worry here. Gustavo