From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f46.google.com ([74.125.82.46]:35181 "EHLO mail-wm0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753052AbbLFQHo (ORCPT ); Sun, 6 Dec 2015 11:07:44 -0500 Received: by wmuu63 with SMTP id u63so113786515wmu.0 for ; Sun, 06 Dec 2015 08:07:43 -0800 (PST) Message-ID: <56645D4D.7060804@electrozaur.com> Date: Sun, 06 Dec 2015 18:07:41 +0200 From: Boaz Harrosh MIME-Version: 1.0 To: Al Viro CC: osd-dev@open-osd.org, linux-fsdevel@vger.kernel.org Subject: Re: [RFC][exofs] locking for sbi->s_nextid? References: <20151205235612.GC20997@ZenIV.linux.org.uk> In-Reply-To: <20151205235612.GC20997@ZenIV.linux.org.uk> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 12/06/2015 01:56 AM, Al Viro wrote: > Looks like exofs_new_inode() does this > > inode->i_ino = sbi->s_nextid++; > > without any locking; sure, the parent directory is locked, but that's not > worth much on a filesystem that supports mkdir()... Am I missing something > subtle here? Yes I guess you are right. What bugs me is why this never failed. I mean on a 64 bit system, why I never get a duplicate ino? But I guess I should change this to an atomic. > Another question in the code nearby: > > ret = ore_get_io_state(&sbi->layout, &oi->oc, &ios); > if (unlikely(ret)) { > EXOFS_ERR("exofs_new_inode: ore_get_io_state failed\n"); > return ERR_PTR(ret); > } > > aren't we leaking a struct inode here? Path around ore_create() is > also interesting - looks like its failure causes a leak (at least if > it happens early on)... > Yes I'm afraid you are absolutely right. Just to show how much attention this toy lab FS ever got. All ore_get_io_state needs to to fail is an OOM. So this was never heavily tested right? I'll see if I have some time to fix both. Just for the fun Thanks Al Boaz