From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread Date: Fri, 10 Nov 2017 08:19:38 +0100 Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 4D24B266F93 for ; Fri, 10 Nov 2017 08:19:41 +0100 (CET) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Arnd Bergmann Cc: jeeja.kp@intel.com, gudishax.kranthikumar@intel.com, alsa-devel@alsa-project.org, Liam Girdwood , Baolin Wang , Vinod Koul , hardik.t.shah@intel.com, Ingo Molnar , guneshwor.o.singh@intel.com, Linux Kernel Mailing List , Fabian Frederick , Mark Brown , Dan Carpenter , Naveen M , Arvind Yadav , Takashi Sakamoto , subhransu.s.prusty@intel.com, SF Markus Elfring , Bhumika Goyal List-Id: alsa-devel@alsa-project.org On Fri, 10 Nov 2017 00:20:10 +0100, Arnd Bergmann wrote: > > On Thu, Nov 9, 2017 at 7:11 PM, Takashi Iwai wrote: > > On Thu, 09 Nov 2017 18:01:47 +0100, > > Arnd Bergmann wrote: > >> > >> On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai wrote: > >> > > >> > IOW, is there any macro indicating the 64bit user time_t? > >> > >> There is a macro defined by the C library, but so far we have not > >> started relying on it in kernel headers, because there is no guarantee > >> that this symbol is visible before sys/time.h has been included, > >> and there are some cases where it's possible to include a kernel > >> header before sys/time.h. > >> > >> In case of sound/asound.h, that should be no problem since we rely > >> on having seen the definition on 'struct timeval' already today, and > >> that must come from sys/time.h. Then we just need to make sure that > >> all C libraries define the same macro. > >> > >> Are you sure about the switch()/case problem? I thought that worked > >> in C99, the only problem would be using the macro outside of a > >> function, e.g. as initalizer for a variable > > > > Hmm, OK it seems working. > > > > But, honestly speaking, it's too scaring. I'm OK if it were only in > > the kernel local code. But it's the API/ABI definition, which is > > referred by user-space... > > > > A more solid condition would be really appreciated. > > I understand your concern here and agree it's really ugly. It did take us > many attempts to come up with this trick for other cases, so my initial > reaction would be to use the same thing everywhere since I know > it works, but we can use #ifdef instead if you prefer that. I think we > can use a single #ifdef variant to cover all cases, but I'd have to think > about the x32 and x86-32 some more here. With this trick, we can > make user space with new glibc use data structures that are compatible > with 64-bit kernels and avoid the additional translation helpers: > > enum { > SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000, > SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000, > #if (__BITS_PER_LONG == 64) || !defined(__USE_TIME_BITS64) Yeah, it's definitely better, more understandable! thanks, Takashi From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755251AbdKJHTn (ORCPT ); Fri, 10 Nov 2017 02:19:43 -0500 Received: from mx2.suse.de ([195.135.220.15]:36287 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750994AbdKJHTm (ORCPT ); Fri, 10 Nov 2017 02:19:42 -0500 Date: Fri, 10 Nov 2017 08:19:38 +0100 Message-ID: From: Takashi Iwai To: Arnd Bergmann Cc: Baolin Wang , Jaroslav Kysela , Fabian Frederick , Arvind Yadav , Linux Kernel Mailing List , alsa-devel@alsa-project.org, Vinod Koul , hardik.t.shah@intel.com, guneshwor.o.singh@intel.com, Liam Girdwood , SF Markus Elfring , gudishax.kranthikumar@intel.com, Mark Brown , Bhumika Goyal , Naveen M , jeeja.kp@intel.com, Takashi Sakamoto , subhransu.s.prusty@intel.com, Ingo Molnar , Dan Carpenter Subject: Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Nov 2017 00:20:10 +0100, Arnd Bergmann wrote: > > On Thu, Nov 9, 2017 at 7:11 PM, Takashi Iwai wrote: > > On Thu, 09 Nov 2017 18:01:47 +0100, > > Arnd Bergmann wrote: > >> > >> On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai wrote: > >> > > >> > IOW, is there any macro indicating the 64bit user time_t? > >> > >> There is a macro defined by the C library, but so far we have not > >> started relying on it in kernel headers, because there is no guarantee > >> that this symbol is visible before sys/time.h has been included, > >> and there are some cases where it's possible to include a kernel > >> header before sys/time.h. > >> > >> In case of sound/asound.h, that should be no problem since we rely > >> on having seen the definition on 'struct timeval' already today, and > >> that must come from sys/time.h. Then we just need to make sure that > >> all C libraries define the same macro. > >> > >> Are you sure about the switch()/case problem? I thought that worked > >> in C99, the only problem would be using the macro outside of a > >> function, e.g. as initalizer for a variable > > > > Hmm, OK it seems working. > > > > But, honestly speaking, it's too scaring. I'm OK if it were only in > > the kernel local code. But it's the API/ABI definition, which is > > referred by user-space... > > > > A more solid condition would be really appreciated. > > I understand your concern here and agree it's really ugly. It did take us > many attempts to come up with this trick for other cases, so my initial > reaction would be to use the same thing everywhere since I know > it works, but we can use #ifdef instead if you prefer that. I think we > can use a single #ifdef variant to cover all cases, but I'd have to think > about the x32 and x86-32 some more here. With this trick, we can > make user space with new glibc use data structures that are compatible > with 64-bit kernels and avoid the additional translation helpers: > > enum { > SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000, > SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000, > #if (__BITS_PER_LONG == 64) || !defined(__USE_TIME_BITS64) Yeah, it's definitely better, more understandable! thanks, Takashi