From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756379AbcBBU2O (ORCPT ); Tue, 2 Feb 2016 15:28:14 -0500 Received: from mail-qk0-f193.google.com ([209.85.220.193]:34874 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750793AbcBBU2M (ORCPT ); Tue, 2 Feb 2016 15:28:12 -0500 Date: Tue, 2 Feb 2016 18:28:07 -0200 From: Gustavo Padovan To: Maarten Lankhorst Cc: 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 v2 06/11] staging/android: turn fence_info into a __u64 pointer Message-ID: <20160202202807.GA2808@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: <1454419402-10769-1-git-send-email-gustavo@padovan.org> <1454419402-10769-7-git-send-email-gustavo@padovan.org> <56B0BA29.1040003@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56B0BA29.1040003@linux.intel.com> 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 Hi Maarten, 2016-02-02 Maarten Lankhorst : > Op 02-02-16 om 14:23 schreef Gustavo Padovan: > > From: Gustavo Padovan > > > > Making fence_info a pointer enables us to extend the struct in the future > > without breaking the ABI. > > > > v2: use type __u64 for fence_info > > 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..03b1214 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..8e2ed32 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; > > > With the information from the commit I would expect fence_info would be a true pointer. I missed that. It is fixed for v3 now. > > so if (sync_file_info->num_fences == 0) not copying fence_info, but set all members including this one. > else for_all_fences() copy_to_user(fence_info struct) > > Otherwise you still can't extend the struct without breaking the abi. all sync_files have at least 1 fence so we are left to the else case, coupy all fences to the user. So we always have variable length at the end of the struct. But I still don't see how this will prevent us from breaking the API. Gustavo