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 X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B91AC33CA2 for ; Wed, 8 Jan 2020 12:51:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id BAA69206F0 for ; Wed, 8 Jan 2020 12:51:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hLiKPkUt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BAA69206F0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 132BE8E000E; Wed, 8 Jan 2020 07:51:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0BC648E0001; Wed, 8 Jan 2020 07:51:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EED108E000E; Wed, 8 Jan 2020 07:51:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0064.hostedemail.com [216.40.44.64]) by kanga.kvack.org (Postfix) with ESMTP id D65A98E0001 for ; Wed, 8 Jan 2020 07:51:51 -0500 (EST) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with SMTP id 6D99F824999B for ; Wed, 8 Jan 2020 12:51:51 +0000 (UTC) X-FDA: 76354454022.24.spoon31_848e24a0bdf32 X-HE-Tag: spoon31_848e24a0bdf32 X-Filterd-Recvd-Size: 8329 Received: from mail-il1-f196.google.com (mail-il1-f196.google.com [209.85.166.196]) by imf35.hostedemail.com (Postfix) with ESMTP for ; Wed, 8 Jan 2020 12:51:50 +0000 (UTC) Received: by mail-il1-f196.google.com with SMTP id c4so2554088ilo.7 for ; Wed, 08 Jan 2020 04:51:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KoaIDjMtfMx7iFf4ZorB4WtDFR0gr8MkRHrKgJexWwg=; b=hLiKPkUtFn1/5/lLfRIcy+j7HXeYe8a3o7U70uqmxm3E1ayJmz04/apx3Te5JbTq0G AJVA7tExTtt3emwiSBxIkKVrYlJgINpP4RMjNbD1Oiy69SF7W2NdMLQXIYki40v8dSqW ZjkwfyWLL6lQJcussns3YaLVR9XSADmnRvZeTfNIWxhRrHRdsXG2mIFlICY23lyilkwQ IHapSxcMC6oeQ0f2B2j4blc8vasFA+l64xiB2Gl4oZ7tMFynrAJNcMjRrgwnEXmjExl3 2gH+MJfAm3v/T+jFPbRbuxMOg6e2oG+f9wOO++RA5DopewXV3rXBQfd9GOgJNPv3/Bv3 h3Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KoaIDjMtfMx7iFf4ZorB4WtDFR0gr8MkRHrKgJexWwg=; b=tzh5lzgR3+tSESIq0b0GsmGK07fcNrueZpAwy6eIGnRyGh5gyFnUtmjtBR5BaekiXN Qo87C2kA81otFbT+tdm/DPOxhpt2KlRiaJrigTh8s4h3nlHvrX8ITd8eEF8a67djDF6P Yhj5IJ+9gal3AJKDJZ4BR2wPl1UNyziKPV66tTTjBN+pISczzM6pB/wXFj0UugHRTgdo QLrsNUeqFYFSrxzKwT4Jv2eZ6CG+MgotERRlKYLD//BeyhQ6SDHQdL5G+Qr1xSi4BHn9 XpUH048CfOOqLE2jVdBlFvpVmt2As8VUZRFi2n/o9qyu2/80xmSoiOlKFC01Ts6bHf5G 29EA== X-Gm-Message-State: APjAAAXs97yhnbB8O3pirMIrI2SCt+rNVH6BB8LnzVOYIZFAfolspOGQ 1tdEDuacRQ6fe+bTKFR47m/yK1Spwo5w7qsVTSQ= X-Google-Smtp-Source: APXvYqyVxXdQ4lE11VQNXo9UW7yajcdnsw/GN2pOs7RWOiS1vc7hOFZMtg2HZIEG/W6sdMncglvNtiNeRgmOE2Vihys= X-Received: by 2002:a92:1711:: with SMTP id u17mr3759500ill.72.1578487910040; Wed, 08 Jan 2020 04:51:50 -0800 (PST) MIME-Version: 1.0 References: <91b4ed6727712cb6d426cf60c740fe2f473f7638.1578225806.git.chris@chrisdown.name> <4106bf3f-5c99-77a4-717e-10a0ffa6a3fa@huawei.com> In-Reply-To: From: Amir Goldstein Date: Wed, 8 Jan 2020 14:51:38 +0200 Message-ID: Subject: Re: [PATCH v5 1/2] tmpfs: Add per-superblock i_ino support To: Hugh Dickins Cc: Chris Down , "zhengbin (A)" , "J. R. Okajima" , Linux MM , Andrew Morton , Al Viro , Matthew Wilcox , Jeff Layton , Johannes Weiner , Tejun Heo , linux-fsdevel , linux-kernel , kernel-team@fb.com Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Jan 8, 2020 at 12:59 PM Hugh Dickins wrote: > > On Tue, 7 Jan 2020, Amir Goldstein wrote: > > > > I vote in favor or best of both patches to result in a simpler outcome: > > 1. use inop approach from Hugh's patch > > 2. use get_next_ino() instead of per-sb ino_batch for kern_mount > > > > Hugh, do you have any evidence or suspect that shmem_file_setup() > > could be contributing to depleting the global get_next_ino pool? > > Depends on what the system is used for: on one system, it will make > very little use of that pool, on another it will be one of the major > depleters of the pool. Generally, it would be kinder to the other > users of the pool (those that might also care about ino duplication) > if shmem were to cede all use of it; but a bigger patch, yes. > > > Do you have an objection to the combination above? > > Objection would be too strong: I'm uncomfortable with it, and not > tempted to replace our internal implementation by one reverting to > use get_next_ino(); but not as uncomfortable as I am with holding > up progress on the issue. > > Uncomfortable because of the "depletion" you mention. Uncomfortable > because half the reason for ever offering the unlimited "nr_inodes=0" > mount option was to avoid stat_lock overhead (though, for all I know, > maybe nobody anywhere uses that option now - and I'll be surprised if > 0-day uses it and reports any slowdown). > > Also uncomfortable because your (excellent, and I'd never thought > about it that way) argument that the shm_mnt objects are internal > and unstatable (that may not resemble how you expressed it, I've not > gone back to search the mails to find where you made the point), is > not entirely correct nowadays - memfd_create()d objects come from > that same shm_mnt, and they can be fstat()ed. What is the > likelihood that anything would care about duplicated inos of > memfd_create()d objects? Fairly low, I think we'll agree; and > we can probably also say that their inos are an odd implementation > detail that no memfd_create() user should depend upon anyway. But > I mention it in case it changes your own feeling about the shm_mnt. > I have no strong feeling either. I just didn't know how intensive its use is. You have provided sufficient arguments IMO to go with your version of the patch. > > > Not-Yet-Signed-off-by: Hugh Dickins > > > > > > 1) Probably needs minor corrections for the 32-bit kernel: I wasn't > > > worrying about that at the time, and expect some "unsigned long"s > > > I didn't need to change for the 64-bit kernel actually need to be > > > "u64"s or "ino_t"s now. > > > 2) The "return 1" from shmem_encode_fh() would nowadays be written as > > > "return FILEID_INO32_GEN" (and overlayfs takes particular interest > > > in that fileid); yet it already put the high 32 bits of the ino into > > > the fh: probably needs a different fileid once high 32 bits are set. > > > > Nice spotting, but this really isn't a problem for overlayfs. > > I agree that it would be nice to follow the same practice as xfs with > > XFS_FILEID_TYPE_64FLAG, but as one can see from the private > > flag, this is by no means a standard of any sort. > > > > As the name_to_handle_at() man page says: > > "Other than the use of the handle_bytes field, the caller should treat the > > file_handle structure as an opaque data type: the handle_type and f_handle > > fields are needed only by a subsequent call to open_by_handle_at()." > > > > It is true that one of my initial versions was checking value returned from > > encode_fh, but Miklos has pointed out to me that this value is arbitrary > > and we shouldn't rely on it. > > > > In current overlayfs, the value FILEID_INO32_GEN is only used internally > > to indicate the case where filesystem has no encode_fh op (see comment > > above ovl_can_decode_fh()). IOW, only the special case of encoding > > by the default export_encode_fh() is considered a proof for 32bit inodes. > > So tmpfs has never been considered as a 32bit inodes filesystem by > > overlayfs. > > Thanks, you know far more about encode_fh() than I ever shall, so for > now I'll take your word for it that we don't need to make any change > there for this 64-bit ino support. One day I should look back, go > through the encode_fh() callsites, and decide what that "return 1" > really ought to say. > TBH, I meant to say that return 1 shouldn't matter functionally, but it would be much nicer to change it to FILEID_INO64_GEN and define that as 0x81 in include/linux/exportfs.h and actually order the gen word after the inode64 words. But that is independent of the per-sb ino change, because the layout of the file handle has always been [gen,inode64] and never really matched that standard of FILEID_INO32_GEN. I may send a patch to later on to change that. Thanks, Amir.