Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753112AbdLKOBx (ORCPT ); Mon, 11 Dec 2017 09:01:53 -0500 Received: from [195.214.232.23] ([195.214.232.23]:38056 "EHLO US-EXCH2.sw.swsoft.com" rhost-flags-FAIL-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1752470AbdLKOBu (ORCPT ); Mon, 11 Dec 2017 09:01:50 -0500 X-Greylist: delayed 916 seconds by postgrey-1.27 at vger.kernel.org; Mon, 11 Dec 2017 09:01:49 EST Subject: Re: [PATCH 1/4] fs/notify: fdinfo can report unsupported file handles. To: Amir Goldstein , NeilBrown CC: Linus Torvalds , Al Viro , linux-fsdevel , Linux NFS Mailing List , lkml , Lennart Poettering References: <151297214390.7818.7216826079527521005.stgit@noble> <151297224512.7818.18333908109878259066.stgit@noble> From: Pavel Emelyanov Message-ID: Date: Mon, 11 Dec 2017 16:46:11 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-ClientProxiedBy: US-EXCH2.sw.swsoft.com (172.16.10.60) To US-EXCH2.sw.swsoft.com (172.16.10.60) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2022 Lines: 48 On 12/11/2017 10:05 AM, Amir Goldstein wrote: > On Mon, Dec 11, 2017 at 8:41 AM, Amir Goldstein wrote: >> On Mon, Dec 11, 2017 at 8:04 AM, NeilBrown wrote: >>> If a filesystem does not set sb->s_export_op, then it >>> does not support filehandles and export_fs_encode_fh() >>> and exportfs_encode_inode_fh() should not be called. >>> They will use export_encode_fh() is which is a default >>> that uses inode number generation number, but in general >>> they may not be stable. >>> >>> So change exportfs_encode_inode_fh() to return FILEID_INVALID >>> if called on an unsupported Filesystem. Currently only >>> notify/fdinfo can do that. >>> >> >> I wish you would leave this check to the caller, maybe add a helper >> exportfs_can_decode_fh() for callers to use. >> >> Although there are no current uses for it in-tree, there is value in >> being able to encode a unique file handle even when it cannot be >> decoded back to an open file. >> >> I am using this property in my fanotify super block watch patches, >> where the object identifier on the event is an encoded file handle >> of the object, which delegates tracking filesystem objects to >> userspace and prevents fanotify from keeping elevated refcounts >> on inodes and dentries. >> >> There are quite a few userspace tools out there that are checking >> that st_ino hasn't changed on a file between non atomic operations. >> Those tools (or others) could benefit from a unique file handle if >> we ever decide to provide a relaxed version of name_to_handle_at(). >> > > And this change need a clause about not breaking userspace. > > Pavel, > > Will this break any version of criu in the wild? If there's no fliehandle in the output, it will make dump fail, but we're already prepared for the fact, that there's no handle at hands. In the worst case criu will exit with error. I also agree that it should only happen when current is OOM killed, and in case of CRIU this means killing criu process itself. -- Pavel