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.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 0EB88C3A59B for ; Fri, 30 Aug 2019 16:39:24 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D6CA62087F for ; Fri, 30 Aug 2019 16:39:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="Rw7TDZKu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D6CA62087F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id A0DC1874A2; Fri, 30 Aug 2019 16:39:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XBQYMO3MwfSD; Fri, 30 Aug 2019 16:39:23 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0C500874AD; Fri, 30 Aug 2019 16:39:23 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by ash.osuosl.org (Postfix) with ESMTP id A36621BF844 for ; Fri, 30 Aug 2019 16:39:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9C9B7893A8 for ; Fri, 30 Aug 2019 16:39:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dzc6SA9NITs2 for ; Fri, 30 Aug 2019 16:39:21 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by hemlock.osuosl.org (Postfix) with ESMTPS id 2093A856B6 for ; Fri, 30 Aug 2019 16:39:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=7hicMWakuRgs2TegBlYfwBT8iDprxzvg2SB2m6q1kQA=; b=Rw7TDZKu0oaHU45vECHr0BFIb QCsALEEmmqL6rPuPL/LqiIl68Hknd4ElB/B166cyi03n5AYPEP585r85yRxsXQlGrftxyqJLVT8kY oRdwaT7wD8Ho9SgDQ1/Y2TgrPgEocQPCGElTwPJ5cOtRQ8Y/6pD6y4/B3j6nh6lkNcvchE+l1mkhR hRKYJtKpSy2gGiNJCMIbnDF7tS6LfuVSauP6kIaz1gPpANUw+xYUPe0Cda6+eBd/31Riixe9Pc+8d GGOCuDkCQkPBO9ka4u10YGEG3XHCnlJ44NCL7WPiMEPGMSbeIgSRUTO1xa2Ww78gWyhWxluJjW6+U ECqizR7OQ==; Received: from hch by bombadil.infradead.org with local (Exim 4.92 #3 (Red Hat Linux)) id 1i3jva-0001L9-Az; Fri, 30 Aug 2019 16:39:10 +0000 Date: Fri, 30 Aug 2019 09:39:10 -0700 From: Christoph Hellwig To: Gao Xiang Subject: Re: [PATCH v6 03/24] erofs: add super block operations Message-ID: <20190830163910.GB29603@infradead.org> References: <20190802125347.166018-1-gaoxiang25@huawei.com> <20190802125347.166018-4-gaoxiang25@huawei.com> <20190829101545.GC20598@infradead.org> <20190829105048.GB64893@architecture4> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190829105048.GB64893@architecture4> User-Agent: Mutt/1.11.4 (2019-03-13) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jan Kara , Chao Yu , Dave Chinner , LKML , Miao Xie , devel@driverdev.osuosl.org, Stephen Rothwell , "Darrick J . Wong" , Christoph Hellwig , Linus Torvalds , Amir Goldstein , Alexander Viro , Jaegeuk Kim , Theodore Ts'o , Greg Kroah-Hartman , David Sterba , Li Guifu , Fang Wei , Pavel Machek , linux-fsdevel@vger.kernel.org, Andrew Morton , linux-erofs@lists.ozlabs.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" List-Archive: On Thu, Aug 29, 2019 at 06:50:48PM +0800, Gao Xiang wrote: > > Please use an erofs_ prefix for all your functions. > > It is already a static function, I have no idea what is wrong here. Which part of all wasn't clear? Have you looked at the prefixes for most functions in the various other big filesystems? > > > + /* 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 3) remove the comment quoted above as it is more confusing than not having the comment > > Is there any good reasons to use buffer heads like this in new code > > vs directly using bios? > > This page can save in bdev page cache, it contains not only the erofs > superblock so it can be fetched in page cache later. If you want it in the page cache why not use read_mapping_page or similar? > > > +/* set up default EROFS parameters */ > > > +static void default_options(struct erofs_sb_info *sbi) > > > +{ > > > +} > > > > No need to add an empty function. > > Later patch will fill this function. Please only add the function in the patch actually adding the functionality. > > > +} > > > > Why is this needed? You can just free your sb privatte information in > > ->put_super and wire up kill_block_super as the ->kill_sb method > > directly. > > See Al's comments, > https://lore.kernel.org/r/20190720224955.GD17978@ZenIV.linux.org.uk/ With that code it makes sense. In this paticular patch it does not. So please add it only when actually needed. _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel