From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.24]) by sourceware.org (Postfix) with ESMTPS id E08C33858CD1; Sat, 9 Dec 2023 15:13:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E08C33858CD1 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 E08C33858CD1 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=134.134.136.24 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1702134840; cv=pass; b=bfG/66JBtos3aQLX+tMKWTbXdrN8cESZpXUvP3kKw8NUDV25SvM/57qKelpQtPudy5ny3l+5/Tn46s59Y4hHiTUxqsPoVEk2sTnu3uz6ZERycN6w+ZBgncJDCzBBmOAnbmaiy04Ispg3crjNgdYKs+/nJxzPAeTTin8kyM4UtHs= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1702134840; c=relaxed/simple; bh=QdyE4QtGZjtpSo1QW6xgrJfRKPmyyXCmDixZ0VhFbFs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=HE0qhej+xqCBZlsVdCVcLdvJIipj1AVN3gW85apd1omDr5pfOxnZ2fsGVND6/E7pOUouYpM8zBt5j/W5EplGvKWYrhh9qU/qpzCYoIfg0ck3iT1NE3UyS0dCcU1z3JeH5IZwcrytb1kXhTzpGV4jWjgsfj5CFRVJrZ8oQasc5W0= ARC-Authentication-Results: i=2; 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=1702134838; x=1733670838; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=QdyE4QtGZjtpSo1QW6xgrJfRKPmyyXCmDixZ0VhFbFs=; b=h5RndFNKkFiE/QpGEdtIRuB76d+j/9GjXgMC6xPOKEnyrAumFuswaoKB 2lRmSP8m5/rxtRWmgsLS78HE0MBYVoam+3iUplQUqxlqBGKnkfKMHZKPX FFVgiwU+jfsYDsKl1MtxttrDCg+3qNRKek5DMcDXnHQw658g8WEukiHL+ 0w7NZLqEwOHUgtbY60QT6C6It2J6jjoGkvrqjgQd0BM5SaX+I/c17Peg3 SeRNY9t0bpdemt5fHQYsaxDNBwDtLoRW/AxRm6Q1QzJ1TqGqzZm6k8DWL uK44bTfShXDmgUZsaLMVSOpQ2Okb8VJGawI1QiGWjaqnTuTqxLvypXdZR g==; X-IronPort-AV: E=McAfee;i="6600,9927,10919"; a="397307017" X-IronPort-AV: E=Sophos;i="6.04,263,1695711600"; d="scan'208";a="397307017" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Dec 2023 07:13:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10919"; a="801432474" X-IronPort-AV: E=Sophos;i="6.04,263,1695711600"; d="scan'208";a="801432474" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by orsmga008.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 09 Dec 2023 07:13:55 -0800 Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Sat, 9 Dec 2023 07:13:55 -0800 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Sat, 9 Dec 2023 07:13:55 -0800 Received: from NAM02-DM3-obe.outbound.protection.outlook.com (104.47.56.40) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Sat, 9 Dec 2023 07:13:55 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=j78TKyCGgbmKTpMs0oBGeE4dGMCgwHyilxCsFo6rPZgaYCW5B4kqPZ3PCeUPSfDPKk4IZuqPJUJb9XlVGNFWr0YeMW+mfu9mSyenNOeVrrVt8me0/c0/ubMV7kbkWvuLBUUSwORQIJMeo5bOG2bEfsSf9sdr7ZlEca5NTMr1nY6jvNTs46anxwvnUu89pLLJWr+5bn2DTVHWxDHBQFPrxBtHXEJp6dlWYMtolv2BdDr0yQid9Y6zbBGXJuXXetodMSwiB8GFe5pcIIDdUOSlsSi5Jq6/OmjVjmfRS4BwGft8ABMt3O9wBpY6iqxuhzKLoBFRxa0K/BbN8Z+NjgIhaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Z6LSlVNKU5uvxNYzN+oZ8qBDmHORzz45yjbBk/sEq0M=; b=MD8uggBV60PtmO7GYr9U/U7+s+Y0RN2wJ5w+r/OMKRBPEUop41R2KYJx5OqlsJdfqlP4RhuRL6aeOc1bo6RD68GWEFPOZHRs+tPpU5zArElNNMUSzk62IwltnDHFBpPrZBMcOc2Eb93AtixX/xVJdvPPQW16X0BXg83RthglpHZHFtOlxwYfMRx5DNgzdnsqA3uo4bpjEgTW8dfgtYOA2R5lvVL7AQ7cxs/Jl+uGDn4b4fpdbCKjc0jlFf6W0eud7JOXPrduZwYItSHB+rf8KBHD/dNLfLAIeqsxWnoa/O+ad6c7gP/Fii5ETUFWiybO0N1Ge84CE5CqsN+vi/EOJA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from PH7PR11MB6056.namprd11.prod.outlook.com (2603:10b6:510:1d4::20) by MW4PR11MB5774.namprd11.prod.outlook.com (2603:10b6:303:182::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7068.29; Sat, 9 Dec 2023 15:13:52 +0000 Received: from PH7PR11MB6056.namprd11.prod.outlook.com ([fe80::a74b:825d:7dc3:67]) by PH7PR11MB6056.namprd11.prod.outlook.com ([fe80::a74b:825d:7dc3:67%3]) with mapi id 15.20.7068.029; Sat, 9 Dec 2023 15:13:52 +0000 From: "Zhu, Lipeng" To: Jakub Jelinek CC: "tkoenig@netcologne.de" , "rep.dot.nop@gmail.com" , "fortran@gcc.gnu.org" , "gcc-patches@gcc.gnu.org" , "Lu, Hongjiu" , "Li, Tianyou" , "Deng, Pan" , "Guo, Wangyang" Subject: RE: [PATCH v6] libgfortran: Replace mutex with rwlock Thread-Topic: [PATCH v6] libgfortran: Replace mutex with rwlock Thread-Index: AQHZ0YJCBPOsNm3ciEG5D5v6nOj66rCf3QyAgAHgkNA= Date: Sat, 9 Dec 2023 15:13:52 +0000 Message-ID: References: <93b9e2d5-4355-136a-a961-da1ae9c1468f@netcologne.de> <20230818031818.2161842-1-lipeng.zhu@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: PH7PR11MB6056:EE_|MW4PR11MB5774:EE_ x-ms-office365-filtering-correlation-id: 1b0c3504-8ccf-4206-7040-08dbf8c97448 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Rn0Ij9LZdBxMJj4RRyn3U4FSrPnhXCG3FzwumN7uxlKUXrcJhidCF/LQywv6PBs5OWzwIXtDcx6KRzQrMwu1xQGhMR/xN7oKM+8EDTRX98zhzylUaeRJa0rJAHPcPWwUbH9y9YzqJsjihbJOrYVprN+Klh+jBYlHO2x9A8OddZqVNzn3xiQ9S6zKtkLQDNLoPLMyuG3PogGsiY2lQR494SpPwDZj1t+bGNLZAkbpj/xY+Z1gpkHG47LZ9QfngEEIHxGeHXk2guzOb6BMlhZbSeMRbguOpdzvv2KOr44ZZwYXMVoSenu/DSG8uMPqwa1Vzd8OszK93m3UrlcLMg0iP5otNIikQvac7tGEeYRCcXHJmyNmHLFH59taFZ0Dik4RlFMeQ10gYlRWy16YRhQ0sX8yE6trhk0Nptpf2gRB6QsBrmYq6vrMlaS0HvqQMSGU7afCsNlKL7OjruATOs6okemL4z5HxEc0b7oKJMY+bS3HuqcWovCZDLdwCD3SiN/D6vyEDPVxDC2C4WNDfbcUMrBGeguamgeL3geVTKaKK/t6kV5ToOQElFKo0wmO6Mvrfx89EU+qSgrZck+hxKeXtqqz0eBnQy0MqgtYuUQgp1o= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PH7PR11MB6056.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(376002)(346002)(396003)(366004)(39860400002)(136003)(230922051799003)(451199024)(64100799003)(1800799012)(186009)(66556008)(66476007)(66446008)(76116006)(66946007)(64756008)(54906003)(7696005)(6506007)(9686003)(53546011)(41300700001)(107886003)(71200400001)(38070700009)(26005)(86362001)(38100700002)(122000001)(82960400001)(33656002)(83380400001)(966005)(478600001)(8936002)(52536014)(8676002)(4326008)(2906002)(5660300002)(316002)(6916009)(55016003);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?maYHElUrx5i1+SmVwBWR49O6EeWLm1kFCOFxDrXJVeu27/BD5xkgfLO6MlfG?= =?us-ascii?Q?oG3QjbTo76f+lxP0pdGYvK+Cg5uVn1ULWjMSC0lK/Ra9Fuea+6NP3sJkn+yQ?= =?us-ascii?Q?J8nIvDiZ3tkaBnti2ecZFsNAhg++pMUkrI1tNuXbywqkmtKmvj95RKuZEOS0?= =?us-ascii?Q?Dt523T1xJoTbHJM4KJ5E2TmZ2XO9l9A/dV7tMBB84jkbcddFce2F9frvMjBE?= =?us-ascii?Q?RdHSsWc8QTJo0efeKxsqWCZdV07yWjhE9C2gN5r9X6pDRY2FQhN3RqP1tGPx?= =?us-ascii?Q?Dt9zCJawrukYugc5zYOkygvypxQElrbMkk1Lhm1FLyTVaY3BYsGOycCNX+9a?= =?us-ascii?Q?YtLxmlaO9fu9PkugbR6OPNR63Fk0g/Ij21TdYzc/s5XvbAKoDq7Mhe+cIq49?= =?us-ascii?Q?097WBEffI1HkuDBrNpVxiXOUCTjBzIvFA4OUg5AKWWw+maZs3H02VHmLVfwV?= =?us-ascii?Q?MA4I1gwWFjiHtW+BbnmwNLojbwINXVHUHz9VcRzU3YK8cNy6tauTYC+1RDGU?= =?us-ascii?Q?nQ6ZwFrUHrqEE9OYxWAfVlQ1UECtCsOpXfAfGcXx+M5o/KaiRFBkJFi5lUc1?= =?us-ascii?Q?5aSw4N8wYWEPzYsXrNy5aGoHRskYksQpXIxgOLXjBiv2to/R5ug6VBOn5cNG?= =?us-ascii?Q?E1wcUrwyVFGigyIJ2zo35tMx/bpTin6iCqeaZpPaqchXaMLJ+BIv35OC9UNk?= =?us-ascii?Q?Y8wSww0CUB+kHhkoSY3KLfmNwj7CQUgxJ755MBUT4qChlFKHd7SZDJUYN68W?= =?us-ascii?Q?m5Y32AHqC6nM0cx0uEdSKXJ96Ds66lpz1u1TB15jEqkGn3O3CwmOvs9SvqFy?= =?us-ascii?Q?pzJ0lt72qcp2xIiLmhWBuIqPvrNDcs22AK+X39XNL21KsNgFme/V3mRGzjeA?= =?us-ascii?Q?9edwSz8jsc2vLM48lodBgH9PqELC/hPdOWfW2GT1x7L1tQCJwZUu7nCjCw9f?= =?us-ascii?Q?fVYky39JqlgB40cmf2zzxJc9E8VkU8InzgXpuETmS/n9BXlDCVo2V88aIM5t?= =?us-ascii?Q?Dqo3FQJpkYtr4j8ep5BSc1YrhnmLu2XoAWhjJ1S8GrmsZ+5FsGFy104y7alB?= =?us-ascii?Q?K+KFDGRJ3BhkCb+izaOc6XnF+tRPGKL9mPN7jA6ZRy9xUjfmJi0DxoYmgeP7?= =?us-ascii?Q?iuclVQhE4ovMrGEFtGz7xmBiT43NWDt2u/NvrnmWSe9OCxXDSXufLiBOfBka?= =?us-ascii?Q?ptDKlBqyto9UVyTLHde1zUFMOmCaBY9JkWbxARV4nj3lP0bbyLfWg1/7U238?= =?us-ascii?Q?u+0qmP3k1NrVbSSL/9V3aClHMHMPSnrDmeO2DB/WNwth6gZXKAkVi/a9G+4i?= =?us-ascii?Q?hv0P58qpLXzGd/ZvwsiQ0D7FZs0nP508DaE3vpLunMkmCZGk+V3hpDE6dtFU?= =?us-ascii?Q?HKDgZ/tapwdY2i/pCcfmTw9bRwdfwkDHm/jZ+8UZoYuW0K+Ln9Ey0kIZmfgT?= =?us-ascii?Q?BDg3p+hYo1Wcb+mvR5mFKzRkMjqhKUdqf/omB2rEHwqScKDYk1eXdU5lIfQJ?= =?us-ascii?Q?aTj1UY/eY0D/voYTqn4nNu77J803aVLZsK7DQ4zQWNz5FY9de7MCIr64t45x?= =?us-ascii?Q?o/ivqiEhH9rEBw8pM+j9DsCtgRbGahGn9eB+jRwy?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB6056.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1b0c3504-8ccf-4206-7040-08dbf8c97448 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Dec 2023 15:13:52.5704 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: mCPoUXeX5IfwEcSkBffPUrgYtSti4uY20vaOcYdOMHeywYIbu/fXsxlZwU+T9x+XC81YO7kqq5KMXytUfT18bA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB5774 X-OriginatorOrg: intel.com X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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/8 18:19, Jakub Jelinek wrote: > On Fri, Aug 18, 2023 at 11:18:19AM +0800, Zhu, Lipeng wrote: > > From: Lipeng Zhu > > > > 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 > > * 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 (internal_proto): Define unit_rwlock > > * io/transfer.c (st_read_done_worker): Relace unit_lock with unit_r= wlock > > (st_write_done_worker): Relace unit_lock with unit_rwlock > > * io/unit.c (get_gfc_unit): Relace unit_lock with unit_rwlock > > (if): Relace unit_lock with unit_rwlock > > (close_unit_1): Relace unit_lock with unit_rwlock > > (close_units): Relace unit_lock with unit_rwlock > > (newunit_alloc): Relace unit_lock with unit_rwlock > > * io/unix.c (flush_all_units): Relace unit_lock with unit_rwlock >=20 > The changeLog entries are all incorrect. > 1) they should be indented by a tab, not 4 spaces, and should end with > a dot > 2) when several consecutive descriptions have the same text, especially > when it is long, it should use Likewise. for the 2nd and following > 3) (internal_proto) is certainly not what you've changed, from what I can > see in io.h you've done: > * 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. > (if) is certainly not what you've changed either, always find what > function or macro the change was in, or if you remove something, state > it, if you add something, state it. > 4) all the > Replace unit_lock with unit_rwlock. descriptions only partially match > reality, you've also changed the operations on those variables. >=20 Hi Jakub,=20 Thanks for your help, very appreciated! I just updated the patch according = to your comments. A new version [PATCH V7] is sent out for your review which=20 update the change log and formatting code according to your following=20 comments. Lipeng Zhu > > --- a/libgfortran/io/async.h > > +++ b/libgfortran/io/async.h > > @@ -207,9 +207,132 @@ > > INTERN_LOCK (&debug_queue_lock); > \ > > MUTEX_DEBUG_ADD (mutex); > \ > > INTERN_UNLOCK (&debug_queue_lock); > \ > > - DEBUG_PRINTF ("%s" DEBUG_RED "ACQ:" DEBUG_NORM " %- > 30s %78p\n", aio_prefix, #mutex, mutex); \ > > + DEBUG_PRINTF ("%s" DEBUG_RED "ACQ:" DEBUG_NORM " %- > 30s %78p\n", aio_prefix, #mutex, \ > > + mutex); \ >=20 > Why are you changing this at all? >=20 > > +#define RD_TO_WRLOCK(rwlock) \ > > + RWUNLOCK (rwlock);\ >=20 > At least a space before \ (or better tab >=20 > > +#define RD_TO_WRLOCK(rwlock) \ > > + RWUNLOCK (rwlock);\ >=20 > Likewise. >=20 > > + WRLOCK (rwlock); > > +#endif > > +#endif > > + > > +#ifndef __GTHREAD_RWLOCK_INIT > > +#define RDLOCK(rwlock) LOCK (rwlock) > > +#define WRLOCK(rwlock) LOCK (rwlock) > > +#define RWUNLOCK(rwlock) UNLOCK (rwlock) #define > RD_TO_WRLOCK(rwlock) > > +{} >=20 > do {} while (0) > instead of {} > ? >=20 > > #endif > > > > #define INTERN_LOCK(mutex) T_ERROR (__gthread_mutex_lock, mutex); > > > > #define INTERN_UNLOCK(mutex) T_ERROR (__gthread_mutex_unlock, > mutex); > > > > +#define INTERN_RDLOCK(rwlock) T_ERROR (__gthread_rwlock_rdlock, > > +rwlock); #define INTERN_WRLOCK(rwlock) T_ERROR > > +(__gthread_rwlock_wrlock, rwlock); #define INTERN_RWUNLOCK(rwlock) > > +T_ERROR (__gthread_rwlock_unlock, rwlock); >=20 > Admittedly preexisting issue, but I wonder why the ; at the end, all the = uses of > RDLOCK etc. I've seen were followed by ; >=20 > > --- a/libgfortran/io/unit.c > > +++ b/libgfortran/io/unit.c > > @@ -33,34 +33,36 @@ see the files COPYING3 and COPYING.RUNTIME > > respectively. If not, see > > > > > > /* IO locking rules: > > - UNIT_LOCK is a master lock, protecting UNIT_ROOT tree and UNIT_CACH= E. > > + UNIT_RWLOCK is a master lock, protecting UNIT_ROOT tree and > UNIT_CACHE. >=20 > master rw lock > ? >=20 > > +/* get_gfc_unit_from_root()-- Given an integer, return a pointer > > + to the unit structure. Returns NULL if the unit does not exist, > > + otherwise returns a locked unit. */ >=20 > Formatting, to is indented differently from otherwise, both should be 3 s= paces, > and there should be 2 spaces after ., rather than just one (in both spots= ). >=20 > > + > > +static inline gfc_unit * get_gfc_unit_from_unit_root (int n) >=20 > Formatting, there shouldn't be space after *, but also function name shou= ld be > at the beginning of line, so in this case it should be static inline gfc_= unit * > get_gfc_unit_from_unit_root (int n) >=20 > > +{ > > + gfc_unit *p; > > + int c =3D 0; > > + p =3D unit_root; >=20 > This should be indented by 2 spaces rather than 4. >=20 > > + while (p !=3D NULL) > > + { > > + c =3D compare (n, p->unit_number); > > + if (c < 0) > > + p =3D p->left; > > + if (c > 0) > > + p =3D p->right; > > + if (c =3D=3D 0) > > + break; > > + } > > + return p; >=20 > And the above return p; line as well. >=20 > Jakub