From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 18A41C4332F for ; Sat, 10 Dec 2022 15:03:29 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4NTrk81CHlz3bgW for ; Sun, 11 Dec 2022 02:03:28 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=q1FUtCdu; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=145.40.68.75; helo=ams.source.kernel.org; envelope-from=xiang@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=q1FUtCdu; dkim-atps=neutral Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4NTrjz46lBz2y34 for ; Sun, 11 Dec 2022 02:03:19 +1100 (AEDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id B5ABDB82A7D; Sat, 10 Dec 2022 15:03:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6067BC433EF; Sat, 10 Dec 2022 15:03:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670684594; bh=2fLrbwOuN6Zimw7Ie1lVFqhhUl9wLVKzzcw9NF6mRwo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=q1FUtCdu6/nAXxVsGKf74aCRhxPV7Y2nNTadnbSMVbjYQ299nW5RBRGGfXPtcEIXM kPoOH3gViYilBEc6SnYL+BHsmW0bgevtdqhaOPApClfllazD7AJxpdInhVMy033WOH 1JammBdyt7UHxcF8mBYJEcmmag/qN7dic5+yWpNRCq1Zm4YVQAgz0dNgoCCdPYydH8 XTwR5y1u8itruzPeCR5R7delOIZdZIcTn0zAz+Pz9Gie3AKEthWlES6G+MUc4KTn07 FEBxcQoxUTvq3l91eDAQpXiqmx9d/ZeC2ZThpkGnukOgIOMANaaJzkln0DqAxxVk7f UI7t9t/QWM+XA== Date: Sat, 10 Dec 2022 23:03:09 +0800 From: Gao Xiang To: Khem Raj Subject: Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases Message-ID: Mail-Followup-To: Khem Raj , linux-erofs@lists.ozlabs.org References: <20221208085335.2884608-1-raj.khem@gmail.com> <20221208085335.2884608-2-raj.khem@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-BeenThere: linux-erofs@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development of Linux EROFS file system List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-erofs@lists.ozlabs.org Errors-To: linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Sender: "Linux-erofs" Hi Khem, On Fri, Dec 09, 2022 at 06:20:12PM -0800, Khem Raj wrote: > On Thu, Dec 8, 2022 at 6:18 AM Gao Xiang wrote: > > > > Hi Khem, > > > > On Thu, Dec 08, 2022 at 12:53:34AM -0800, Khem Raj wrote: > > > erosfs depend on the consistent use of a 64bit offset > > > > Thanks for your patch! > > > > ^ erofs > > Done in v2 > > > > > > type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64), > > > so that it becomes impossible for them to use 32bit interfaces. > > > > > > include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS > > > which was detected by configure. This header needs to be included > > > before any system headers are included to ensure they see the correct > > > definition of _FILE_OFFSET_BITS for the platform > > > > > > Signed-off-by: Khem Raj > > > --- > > > > ... > > > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h > > > index 6a70f11..9cc20a8 100644 > > > --- a/include/erofs/internal.h > > > +++ b/include/erofs/internal.h > > > @@ -12,6 +12,7 @@ extern "C" > > > { > > > #endif > > > > > > +#include > > > > could we use alternative way? since I'd like to make include/ as > > liberofs later, and "config.h" autoconf seems weird to me... > > > > I am using the AC_SYS_LARGEFILE macro from autoconf to enable support for > largefile support during configure. configure will generate config.h > in build dir which > will contain the essential macros which we use e.g. _FILE_OFFSET_BITS defined > to right values. Alternate way is to pass it _always_ or demand it to > be passed from > user. Which in a way it will do with internal.h check added in this > series. I am fine > if you do not want to depend on autoconf support to enable LFS. Let me know. > > > > #include "list.h" > > > #include "err.h" > > > > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h > > > index 08f9761..a3bd93c 100644 > > > --- a/include/erofs_fs.h > > > +++ b/include/erofs_fs.h > > > @@ -9,6 +9,8 @@ > > > #ifndef __EROFS_FS_H > > > #define __EROFS_FS_H > > > > > > +#include > > > > Could you give more hints why we need this here? > > Its needed to get off_t defined, I have added a comment here > in v2i but we don't use off_t in erofs_fs.h? > > > > > + > > > #define EROFS_SUPER_MAGIC_V1 0xE0F5E1E2 > > > #define EROFS_SUPER_OFFSET 1024 > > > > > > @@ -410,6 +412,10 @@ enum { > > > > > > #define EROFS_NAME_LEN 255 > > > > > > + > > > +/* make sure that any user of the erofs headers has atleast 64bit off_t type */ > > > +extern int eros_assert_largefile[sizeof(off_t)-8]; > > > > erofs? also you could add this into erofs/internal.h... > > > > This file is just the on-disk definition... > > yeah moved the check to internal.h in v2 > > > > > > + > > > /* check the EROFS on-disk layout strictly at compile time */ > > > static inline void erofs_check_ondisk_layout_definitions(void) > > > { > > > diff --git a/lib/Makefile.am b/lib/Makefile.am > > > index 3fad357..88400ed 100644 > > > --- a/lib/Makefile.am > > > +++ b/lib/Makefile.am > > > @@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h > > > liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \ > > > namei.c data.c compress.c compressor.c zmap.c decompress.c \ > > > compress_hints.c hashmap.c sha256.c blobchunk.c dir.c > > > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include > > > +liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h > > > > same here too... > > as said above if we are ok to pass it always then we can add -D > _FILE_OFFSET_BITS=64 via toplevel Makefile.am > it will only be needed on 32bit systems though, so maybe we do not > define it and demand it from users via CFLAGS > if they compile it for 32bit systems. > I think use -D _FILE_OFFSET_BITS=64 would be a better choice... Thanks, Gao Xiang