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=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 7B795C3A59B for ; Sat, 31 Aug 2019 00:55:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4F23923431 for ; Sat, 31 Aug 2019 00:55:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728474AbfHaAzj (ORCPT ); Fri, 30 Aug 2019 20:55:39 -0400 Received: from szxga02-in.huawei.com ([45.249.212.188]:3984 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728251AbfHaAzj (ORCPT ); Fri, 30 Aug 2019 20:55:39 -0400 Received: from DGGEMM402-HUB.china.huawei.com (unknown [172.30.72.54]) by Forcepoint Email with ESMTP id 6683374D304E71526AA5; Sat, 31 Aug 2019 08:55:36 +0800 (CST) Received: from dggeme762-chm.china.huawei.com (10.3.19.108) by DGGEMM402-HUB.china.huawei.com (10.3.20.210) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 31 Aug 2019 08:55:35 +0800 Received: from architecture4 (10.140.130.215) by dggeme762-chm.china.huawei.com (10.3.19.108) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1591.10; Sat, 31 Aug 2019 08:55:35 +0800 Date: Sat, 31 Aug 2019 08:54:46 +0800 From: Gao Xiang To: Christoph Hellwig CC: Alexander Viro , Greg Kroah-Hartman , Andrew Morton , Stephen Rothwell , Theodore Ts'o , "Pavel Machek" , David Sterba , Amir Goldstein , "Darrick J . Wong" , "Dave Chinner" , Jaegeuk Kim , Jan Kara , Linus Torvalds , , , LKML , , Chao Yu , Miao Xie , Li Guifu , Fang Wei Subject: Re: [PATCH v6 03/24] erofs: add super block operations Message-ID: <20190831005446.GA233871@architecture4> References: <20190802125347.166018-1-gaoxiang25@huawei.com> <20190802125347.166018-4-gaoxiang25@huawei.com> <20190829101545.GC20598@infradead.org> <20190829105048.GB64893@architecture4> <20190830163910.GB29603@infradead.org> <20190830171510.GC107220@architecture4> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20190830171510.GC107220@architecture4> User-Agent: Mutt/1.9.4 (2018-02-28) X-Originating-IP: [10.140.130.215] X-ClientProxiedBy: dggeme701-chm.china.huawei.com (10.1.199.97) To dggeme762-chm.china.huawei.com (10.3.19.108) X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Christoph, On Sat, Aug 31, 2019 at 01:15:10AM +0800, Gao Xiang wrote: [] > > > > > > > + /* be careful RCU symlink path (see ext4_inode_info->i_data)! */ > > > > > + if (is_inode_fast_symlink(inode)) > > > > > + kfree(inode->i_link); > > > > > > > > is_inode_fast_symlink only shows up in a later patch. And really > > > > obsfucates the check here in the only caller as you can just do an > > > > unconditional kfree here - i_link will be NULL except for the case > > > > where you explicitly set it. > > > > > > I cannot fully understand your point (sorry about my English), > > > I will reply you about this later. > > > > With that I mean that you should: > > > > 1) remove is_inode_fast_symlink and just opencode it in the few places > > using it > > 2) remove the check in this place entirely as it is not needed Add some words about this suggestion since I'm addressing this place, it seems it could not (or I am not sure at least) be freed unconditionally union { struct pipe_inode_info *i_pipe; struct block_device *i_bdev; struct cdev *i_cdev; char *i_link; unsigned i_dir_seq; }; while I saw what shmem did, it seems that they handle as follows: 3636 static void shmem_free_in_core_inode(struct inode *inode) 3637 { 3638 if (S_ISLNK(inode->i_mode)) 3639 kfree(inode->i_link); 3640 kmem_cache_free(shmem_inode_cachep, SHMEM_I(inode)); 3641 } I think that would be some check on it to get it is a symlink (for i_dir_seq it seems unsafe).... I think the original check is ok but I will opencode it instead. Thanks, Gao Xiang