[RFC][exofs] locking for sbi->s_nextid?

[RFC][exofs] locking for sbi->s_nextid?

[Al Viro]

	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?  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)...

Re: [RFC][exofs] locking for sbi->s_nextid?

[Boaz Harrosh]

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