From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.126]) by sourceware.org (Postfix) with ESMTPS id 9C982386186C; Fri, 15 Dec 2023 11:31:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9C982386186C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=intel.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9C982386186C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=134.134.136.126 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702639921; cv=none; b=ldzbjKzEm599e3uFj/dPJbr+KTgylZYH/XdWayfSCFdk9jocqSDu33ZwgcurWXVtDrE879afisqdG8x6M672PdUW2UKA/5ruv0fz1r7h0ByG57WsVDirpLsbOL4VVjuP9udDExoovZ/HZXzC25hCN1kfSAyxzLpsTTbNy+JJDGc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702639921; c=relaxed/simple; bh=yi3RhNaBl+sHf6L9Sw+U/oNU/0n7WZRNEl+eyKPO/CY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=BCPAtoSolGQjjplplY7txheqcv71mjOmtI3/jkxsqv09ctsSyxdcRFP6NPGZptOytxcoMnLzEVp6yTLcnXCl1CwfXRZRCKzFRLSh02xoAuyRmINR0rSTUqEBu06Q0jrEixwfE80vKKQCwLqviF6ln0DG0rCzT3FJlb4h0dLF+/M= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1702639919; x=1734175919; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=yi3RhNaBl+sHf6L9Sw+U/oNU/0n7WZRNEl+eyKPO/CY=; b=nHP5rzVd91R5/6oY7Y8+8bofEbDvxaZvqgcmpE3gNluT8dtH7RzXySG0 O4QAA8WJUH1Tq2A9JnG+cx99kNcf2e5nmF0DeKROy6P9SZLpHaAPimjNI EWC3zmByQkOVcX8vnYHBCBfMVg0P3azTeoD4TWg0BpyUSK7LciXfN6TUN yRpfa9y/qBZMQqCmGlsbdZnDV/dSmZRhaeC3Cz8tbncrTBHZwfVYa2+vH ruStKnrH6CI2damgytpS3ZcaWr4DZTm4mlvpq/loQSsc9op6s+zuXFM1p qNRl5520S7wNi5MybZM9KPhV5y3pnpE2UrnhuwA7qV49/LF9FjQ3xj51r Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10924"; a="380259760" X-IronPort-AV: E=Sophos;i="6.04,278,1695711600"; d="scan'208";a="380259760" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2023 03:31:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.04,278,1695711600"; d="scan'208";a="16264966" Received: from zhulipeng-win.ccr.corp.intel.com (HELO [10.238.0.214]) ([10.238.0.214]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2023 03:31:56 -0800 Message-ID: <8192480a-1dfc-4bc6-b141-0254364d3fba@intel.com> Date: Fri, 15 Dec 2023 19:31:53 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7] libgfortran: Replace mutex with rwlock Content-Language: en-US To: "Richard Earnshaw (lists)" , jakub@redhat.com Cc: fortran@gcc.gnu.org, gcc-patches@gcc.gnu.org, hongjiu.lu@intel.com, pan.deng@intel.com, rep.dot.nop@gmail.com, tianyou.li@intel.com, tkoenig@netcologne.de, wangyang.guo@intel.com References: <20231209153944.3746165-1-lipeng.zhu@intel.com> <523c6fbf-344c-48b8-8508-7e2acb088606@arm.com> From: Lipeng Zhu Organization: Intel In-Reply-To: <523c6fbf-344c-48b8-8508-7e2acb088606@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023/12/14 23:50, Richard Earnshaw (lists) wrote: > On 09/12/2023 15:39, Lipeng Zhu wrote: >> This patch try to introduce the rwlock and split the read/write to >> unit_root tree and unit_cache with rwlock instead of the mutex to >> increase CPU efficiency. In the get_gfc_unit function, the percentage >> to step into the insert_unit function is around 30%, in most instances, >> we can get the unit in the phase of reading the unit_cache or unit_root >> tree. So split the read/write phase by rwlock would be an approach to >> make it more parallel. >> >> BTW, the IPC metrics can gain around 9x in our test >> server with 220 cores. The benchmark we used is >> https://github.com/rwesson/NEAT >> >> libgcc/ChangeLog: >> >> * gthr-posix.h (__GTHREAD_RWLOCK_INIT): New macro. >> (__gthrw): New function. >> (__gthread_rwlock_rdlock): New function. >> (__gthread_rwlock_tryrdlock): New function. >> (__gthread_rwlock_wrlock): New function. >> (__gthread_rwlock_trywrlock): New function. >> (__gthread_rwlock_unlock): New function. >> >> libgfortran/ChangeLog: >> >> * io/async.c (DEBUG_LINE): New macro. >> * io/async.h (RWLOCK_DEBUG_ADD): New macro. >> (CHECK_RDLOCK): New macro. >> (CHECK_WRLOCK): New macro. >> (TAIL_RWLOCK_DEBUG_QUEUE): New macro. >> (IN_RWLOCK_DEBUG_QUEUE): New macro. >> (RDLOCK): New macro. >> (WRLOCK): New macro. >> (RWUNLOCK): New macro. >> (RD_TO_WRLOCK): New macro. >> (INTERN_RDLOCK): New macro. >> (INTERN_WRLOCK): New macro. >> (INTERN_RWUNLOCK): New macro. >> * io/io.h (struct gfc_unit): Change UNIT_LOCK to UNIT_RWLOCK in >> a comment. >> (unit_lock): Remove including associated internal_proto. >> (unit_rwlock): New declarations including associated internal_proto. >> (dec_waiting_unlocked): Use WRLOCK and RWUNLOCK on unit_rwlock >> instead of __gthread_mutex_lock and __gthread_mutex_unlock on >> unit_lock. >> * io/transfer.c (st_read_done_worker): Use WRLOCK and RWUNLOCK on >> unit_rwlock instead of LOCK and UNLOCK on unit_lock. >> (st_write_done_worker): Likewise. >> * io/unit.c: Change UNIT_LOCK to UNIT_RWLOCK in 'IO locking rules' >> comment. Use unit_rwlock variable instead of unit_lock variable. >> (get_gfc_unit_from_unit_root): New function. >> (get_gfc_unit): Use RDLOCK, WRLOCK and RWUNLOCK on unit_rwlock >> instead of LOCK and UNLOCK on unit_lock. >> (close_unit_1): Use WRLOCK and RWUNLOCK on unit_rwlock instead of >> LOCK and UNLOCK on unit_lock. >> (close_units): Likewise. >> (newunit_alloc): Use RWUNLOCK on unit_rwlock instead of UNLOCK on >> unit_lock. >> * io/unix.c (find_file): Use RDLOCK and RWUNLOCK on unit_rwlock >> instead of LOCK and UNLOCK on unit_lock. >> (flush_all_units): Use WRLOCK and RWUNLOCK on unit_rwlock instead >> of LOCK and UNLOCK on unit_lock. >> > > It looks like this has broken builds on arm-none-eabi when using newlib: > > In file included from /work/rearnsha/gnusrc/nightly/gcc-cross/master/libgfortran > /runtime/error.c:27: > /work/rearnsha/gnusrc/nightly/gcc-cross/master/libgfortran/io/io.h: In function > ‘dec_waiting_unlocked’: > /work/rearnsha/gnusrc/nightly/gcc-cross/master/libgfortran/io/io.h:1023:3: error > : implicit declaration of function ‘WRLOCK’ [-Wimplicit-function-declaration] > 1023 | WRLOCK (&unit_rwlock); > | ^~~~~~ > /work/rearnsha/gnusrc/nightly/gcc-cross/master/libgfortran/io/io.h:1025:3: error > : implicit declaration of function ‘RWUNLOCK’ [-Wimplicit-function-declaration] > 1025 | RWUNLOCK (&unit_rwlock); > | ^~~~~~~~ > > > R. Hi Richard, The root cause is that the macro WRLOCK and RWUNLOCK are not defined in io.h. The reason of x86 platform not failed is that HAVE_ATOMIC_FETCH_ADD is defined then caused above macros were never been used. Code logic show as below: #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else WRLOCK (&unit_rwlock); u->waiting--; RWUNLOCK (&unit_rwlock); #endif I just draft a patch try to fix this bug, because I didn't have arm platform, would you help to validate if it was fixed on arm platform? diff --git a/libgfortran/io/io.h b/libgfortran/io/io.h index 15daa0995b1..c7f0f7d7d9e 100644 --- a/libgfortran/io/io.h +++ b/libgfortran/io/io.h @@ -1020,9 +1020,15 @@ dec_waiting_unlocked (gfc_unit *u) #ifdef HAVE_ATOMIC_FETCH_ADD (void) __atomic_fetch_add (&u->waiting, -1, __ATOMIC_RELAXED); #else - WRLOCK (&unit_rwlock); +#ifdef __GTHREAD_RWLOCK_INIT + __gthread_rwlock_wrlock (&unit_rwlock); + u->waiting--; + __gthread_rwlock_unlock (&unit_rwlock); +#else + __gthread_mutex_lock (&unit_rwlock); u->waiting--; - RWUNLOCK (&unit_rwlock); + __gthread_mutex_unlock (&unit_rwlock); +#endif #endif } Lipeng Zhu