From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2e.google.com (mail-oa1-x2e.google.com [IPv6:2001:4860:4864:20::2e]) by sourceware.org (Postfix) with ESMTPS id 589153858D28 for ; Tue, 29 Aug 2023 17:06:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 589153858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x2e.google.com with SMTP id 586e51a60fabf-1cca7cf6e01so2682674fac.0 for ; Tue, 29 Aug 2023 10:06:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1693328792; x=1693933592; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=3UhKuExihcZreAxwyv40rhagONN3e7tQ9J0qgg6Nn/w=; b=cSabIfu5UT+X8VVyZOHK+a0QavElrwXCYF0rCH9DA0I58M0/PgYT+YFpPmItL8Dx9g 6RGjXS85UixdgOwaXI7dBp/hVBvQN+QlwEoJx1rAP1moeEqL6Zu4Dw/1C9Xlm/rJ4Pq9 jyQ48rZPgeOk+3hTaJNvCJ4YnPVq8V/Ff8yznBH9x0sLXsm4rUW5RoEOZ+J9CRwHIw40 TYiToz8aeGbo9pkZSuq1azo43QpeuiJxU0Wz4SaOAdGPRibVno7j+HWh/z/uAwXbtsnS ESySOurzm3UtHmvouJFdtn2qYyaUkFc/Y3Bq+bWVvzyByS7xb5FOkbGuvO7gl5P3P6VT EV1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693328792; x=1693933592; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3UhKuExihcZreAxwyv40rhagONN3e7tQ9J0qgg6Nn/w=; b=lPX5n3vSEM37tAzgyr7oZM/e7s63+hfLqmFtJPNyXgpxafYGvdgB1KnqBvayx2uowv mFUyrqY+fSOV6YP89JkvyiBxUJre7bTEBZj72cPUHgV3TyCmY7kPfXgV6gXtFvl3KK5f xa6FqmE/vw2iLPvh9GJheIsi4uxtiwqt6V9Dbsf3WUwx3QSoLL5t+kysUsDyDgdL8ZKN 9fkF5i4/WbLLw42uZeXv1W2htikJAEIh0iS3+utid6Rofwf0KzEoQC1nHC1MNWUD6qvD mPUzl+PS1cA2WPXnr3TZ5qopvAHIGaDNTCRo5X6SO3d032ig2MoagKXfTSiRSv5CH/pM gzmw== X-Gm-Message-State: AOJu0YwBe3IXN6zI40im1dFX9hPuR+Ph7ZrXZppxpdehRRJ8jC6M9YIV NrTThnmbP+hUVF3nWUI9dqvpknSlH0A5/NpwjltnuQ== X-Google-Smtp-Source: AGHT+IEoYcPK52TXW4jblzqDJv/bcGDxg0FYEhcUH9kgIuRUvcikLx/FDnYMFCm2wp+9k0nZKeyPOw== X-Received: by 2002:a05:6870:ac27:b0:1bf:54b9:800 with SMTP id kw39-20020a056870ac2700b001bf54b90800mr14061767oab.59.1693328792694; Tue, 29 Aug 2023 10:06:32 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c3:578c:7535:3d41:c7e8:c5ab? ([2804:1b3:a7c3:578c:7535:3d41:c7e8:c5ab]) by smtp.gmail.com with ESMTPSA id z7-20020a056870e14700b001d0d4c3f758sm514872oaa.9.2023.08.29.10.06.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 29 Aug 2023 10:06:32 -0700 (PDT) Message-ID: Date: Tue, 29 Aug 2023 14:06:27 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH] io: Fix record locking contants for powerpc64 with __USE_FILE_OFFSET64 Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org, Aurelien Jarno References: <20230828213721.2957677-1-aurelien@aurel32.net> <87pm36nut5.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 29/08/23 13:30, Aurelien Jarno wrote: > On 2023-08-29 10:20, Adhemerval Zanella Netto wrote: >> >> >> On 29/08/23 04:59, Florian Weimer via Libc-alpha wrote: >>> * Aurelien Jarno: >>> >>>> diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/fcntl.h b/sysdeps/unix/sysv/linux/powerpc/bits/fcntl.h >>>> index f7615a447e..d8a291a331 100644 >>>> --- a/sysdeps/unix/sysv/linux/powerpc/bits/fcntl.h >>>> +++ b/sysdeps/unix/sysv/linux/powerpc/bits/fcntl.h >>>> @@ -33,7 +33,7 @@ >>>> # define __O_LARGEFILE 0200000 >>>> #endif >>>> >>>> -#if __WORDSIZE == 64 >>>> +#if __WORDSIZE == 64 && !defined __USE_FILE_OFFSET64 >>>> # define F_GETLK 5 >>>> # define F_SETLK 6 >>>> # define F_SETLKW 7 >>> >>> I find this puzzling. Why would __USE_FILE_OFFSET64 have an effect if >>> __WORDSIZE is 64? >> >> This is a historical artifact from powerpc64. Instead of following other 64-bit >> architectures and define F_GETLK the same whether _FILE_OFFSET_BITS is defined, >> the port used powerpc definitiosn that required different values to support LFS. >> >> This patch is not wrong, but at same time not really required. The powercp64 >> fcntl will handle F_GETLK/F_SETLK/F_SETLKW with the historical values with >> the FCNTL_ADJUST_CMD macro, so old binaries will continue to work as expected. > > It does break some binaries, for instance File-FcntlLock [1] with the > following scenario: > - perl is built against glibc 2.37 > - system is upgraded to glibc 2.38 > - File-FcntlLock is built against glibc 2.38: it does not work as the > value of F_GETLK has changed from 12 to 5 [2]. > > In short these constants are not exclusively used by the glibc. They > might be referenced in some libraries, and changing their values break > things. > > [1] https://metacpan.org/dist/File-FcntlLock > [2] https://metacpan.org/release/JTT/File-FcntlLock-0.22/source/FcntlLock.xs#L74 > Sigh, you are correct. And it is also means that default and LFS objects are essentially incompatible on powerpc64 in this regard for this very reason, but I agree that we should keep the compatibility. Could add a LFS test, just to make sure the lock constants are correct: diff --git a/io/Makefile b/io/Makefile index 6ccc0e8691..8a3c83a3bb 100644 --- a/io/Makefile +++ b/io/Makefile @@ -192,6 +192,7 @@ tests := \ tst-fchownat \ tst-fcntl \ tst-fcntl-lock \ + tst-fcntl-lock-lfs \ tst-fstatat \ tst-fts \ tst-fts-lfs \ diff --git a/io/tst-fcntl-lock-lfs.c b/io/tst-fcntl-lock-lfs.c new file mode 100644 index 0000000000..f2a909fb02 --- /dev/null +++ b/io/tst-fcntl-lock-lfs.c @@ -0,0 +1,2 @@ +#define _FILE_OFFSET_BITS 64 +#include It won't really trigger this issue, but it a extra sanity test.