Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Namjae Jeon" <namjae.jeon@samsung.com>
To:     "'Al Viro'" <viro@zeniv.linux.org.uk>
Cc:     <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <gregkh@linuxfoundation.org>, <valdis.kletnieks@vt.edu>,
        <hch@lst.de>, <sj1557.seo@samsung.com>, <pali.rohar@gmail.com>,
        <arnd@arndb.de>, "'Namjae Jeon'" <linkinjeon@gmail.com>
In-Reply-To: <20200121014924.GI8904@ZenIV.linux.org.uk>
Subject: RE: [PATCH v12 02/13] exfat: add super block operations
Date:   Tue, 21 Jan 2020 11:34:40 +0900
Message-ID: <005201d5d003$55325960$ff970c20$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQGweyIxypJzru68cwvOK1cQ+ZOFYgKRp5m0AvjvcmoCRBuVtKgAmmug
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <20200120124428.17863-1-linkinjeon@gmail.com>
        <20200120124428.17863-3-linkinjeon@gmail.com>
        <CGME20200121014933epcas1p22c54d6162bca6ebe14715542f034993b@epcas1p2.samsung.com>
        <20200121014924.GI8904@ZenIV.linux.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> > +static void exfat_put_super(struct super_block *sb) {
> > +	struct exfat_sb_info *sbi = EXFAT_SB(sb);
> > +
> > +	mutex_lock(&sbi->s_lock);
> > +	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state))
> > +		sync_blockdev(sb->s_bdev);
> > +	exfat_set_vol_flags(sb, VOL_CLEAN);
> > +	exfat_free_upcase_table(sb);
> > +	exfat_free_bitmap(sb);
> > +	mutex_unlock(&sbi->s_lock);
> > +
> > +	if (sbi->nls_io) {
> > +		unload_nls(sbi->nls_io);
> > +		sbi->nls_io = NULL;
> > +	}
> > +	exfat_free_iocharset(sbi);
> > +	sb->s_fs_info = NULL;
> > +	kfree(sbi);
> > +}
> 
> You need to RCU-delay freeing sbi and zeroing ->nls_io.  *Everything* used
> by ->d_compare() and ->d_hash() needs that treatment.  RCU-mode pathwalk
> can stray into a filesystem that has already been lazy-umounted and is
> just one close() away from shutdown.  It's OK, as long as you make sure
> that all structures used in methods that could be called in RCU mode (-
> >d_compare(), ->d_hash(), rcu-case ->d_revalidate(), rcu-case -
> >permission()) have destruction RCU-delayed.  Look at what VFAT is doing;
> that's precisely the reason for that delayed_free() thing in there.
Okay.
> 
> > +static void exfat_destroy_inode(struct inode *inode) {
> > +	kmem_cache_free(exfat_inode_cachep, EXFAT_I(inode)); }
> 
> No.  Again, that MUST be RCU-delayed; either put an explicit
> call_rcu() here, or leave as-is, but make that ->free_inode().
Okay.
> 
> > +static void __exit exit_exfat_fs(void) {
> > +	kmem_cache_destroy(exfat_inode_cachep);
> > +	unregister_filesystem(&exfat_fs_type);
> 
> ... and add rcu_barrier() here.
Okay, I will fix them on next version.
Thanks for review!!
> 
> > +	exfat_cache_shutdown();