From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116674 invoked by alias); 2 May 2018 11:08:55 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 116665 invoked by uid 89); 2 May 2018 11:08:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:HELO, Hx-spam-relays-external:HELO X-HELO: mga02.intel.com X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 Subject: Re: [PATCH v2 1/3] Tunables: Add tunables of spin count for pthread adaptive spin mutex To: Florian Weimer , Adhemerval Zanella , Glibc alpha Cc: Dave Hansen , Tim Chen , Andi Kleen , Ying Huang , Aaron Lu , Lu Aubrey References: <1524624988-29141-1-git-send-email-kemi.wang@intel.com> <0c66f19d-c0e8-accd-85dd-7e55dd6da1af@redhat.com> From: kemi Message-ID: <55c818fb-1b7e-47d0-0287-2ea33ce69fd5@intel.com> Date: Wed, 02 May 2018 11:08:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <0c66f19d-c0e8-accd-85dd-7e55dd6da1af@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2018-05/txt/msg00038.txt.bz2 Hi, Florian Thanks for your time to review. On 2018年05月02日 16:04, Florian Weimer wrote: > On 04/25/2018 04:56 AM, Kemi Wang wrote: > >> +  mutex { >> +    spin_count { >> +      type: INT_32 >> +      minval: 0 >> +      maxval: 30000 >> +      default: 1000 >> +    } > > How did you come up with the default and maximum values?  Larger maximum values might be useful for testing boundary conditions. > For the maximum value of spin count: Please notice that mutex->__data.__spins += (cnt - mutex->__data.__spins) / 8, and the variable *cnt* could reach the value of spin count due to spinning timeout. In such case, mutex->__data.__spins is increased and could be close to *cnt* (close to the value of spin count). Keeping the value of spin count less than MAX_SHORT can avoid the overflow of mutex->__data.__spins variable with the possible type of short. For the default value of spin count: I referred to the previous number of 100 times for trylock in the loop. When this mode is changed to read only while spinning. I suppose the value could be larger because of lower overhead and latency of read compared with cmpxchg. Perhaps we should make the default value of spin count differently according to architecture. >> +# define TUNABLE_CALLBACK_FNDECL(__name, __type)            \ >> +static inline void                        \ >> +__always_inline                            \ >> +do_set_mutex_ ## __name (__type value)            \ >> +{                                \ >> +  __mutex_aconf.__name = value;                \ >> +}                                \ >> +void                                \ >> +TUNABLE_CALLBACK (set_mutex_ ## __name) (tunable_val_t *valp) \ >> +{                                \ >> +  __type value = (__type) (valp)->numval;            \ >> +  do_set_mutex_ ## __name (value);                \ >> +} >> + >> +TUNABLE_CALLBACK_FNDECL (spin_count, int32_t); > > I'm not sure if the macro is helpful in this context. > It is a matter of taste. But, perhaps we have other mutex tunables in future. >> +static void >> +mutex_tunables_init (int argc __attribute__ ((unused)), >> +                  char **argv  __attribute__ ((unused)), >> +                          char **environ) >> +{ >> +#if HAVE_TUNABLES >> + >> +  TUNABLE_GET (spin_count, int32_t, >> +          TUNABLE_CALLBACK (set_mutex_spin_count)); >> +#endif >> +} >> + >> +#ifdef SHARED >> +# define INIT_SECTION ".init_array" >> +#else >> +# define INIT_SECTION ".preinit_array" >> +#endif >> + >> +void (*const __pthread_mutex_tunables_init_array []) (int, char **, char **) >> +  __attribute__ ((section (INIT_SECTION), aligned (sizeof (void *)))) = >> +{ >> +  &mutex_tunables_init >> +}; > > Can't you perform the initialization as part of overall pthread initialization?  This would avoid the extra relocation. > Thanks for your suggestion. I am not sure how to do it now and will take a look at it. > Thanks, > Florian