Re: ext4_orphan_del() sleeps in non-journal mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Sat, Sep 15, 2012 at 3:28 PM, Anatol Pomozov
<anatol.pomozov@xxxxxxxxx> wrote:
> D'oh, sorry.
>
> I did not realize that it is our local modification that calls
> ext4_orphan_del(NULL) from ext4_end_io_dio. So please ignore the issue
> "ext4_orphan_del sleeps in softirq context".
>
> But the other more general issue "ext4_orphan_del sleeps in no-journal
> mode" still applies. As Dmitry mentioned in this commit 3d287de3b828
> such sleep might degrade performance. In no-journal mode we do not
> need to manipulate with i_orphan list and no reason to take the mutex.
>
> I checked all ext4_orphan_del(NULL,...) usages and some of them look
> good for me, e.g.
>
>                 handle = ext4_journal_start(inode, 2);
>                 if (IS_ERR(handle)) {
>                         if (inode->i_nlink)
>                                 ext4_orphan_del(NULL, inode);
>
> In this example we take handle and important thing to note here is
> that IS_ERR(handle) can be true only in journal mode when starting
> journal failed. In non-journal mode ext4_journal_start() *always*
> returns a fake handle that is non-error (see ext4_get_nojournal). So
> the example above never sleeps in ext4_orphan_del().
>
> But there are other examples where sleep might happen (at current HEAD
> 3f0c3c8fe):
>
> inode.c:281
> inode.c:956
> inode.c:1069
> inode.c:1111
> inode.c:1177
>
> I think we need to fix these 5 places. In all other cases we check
> IS_ERR(handle) and thus do not call ext4_orphan_del() in no-journal
> mode.

Actually inode.c:1177 is also fine as we check ext4_handle_valid() in
this function above, thus this codepath can be never called in
no-journal mode. The other 4 places (and plus my local code) have
potential issues.

I am still trying to understand what is the semantics of
"ext4_orphand_del(NULL,...)". Does NULL mean

a) journaling is enabled, an error happened and we do not have a valid
handle anymore but we need to remove an added inode from orphan list.
In this case we should either check "EXT4_SB(sb)->s_journal" *before*
calling ext4_orphand_del, or introduce a flag that indicated the inode
was successfully added (like Dmitry made in 3d287de3).

b) and error happened and we need to remove inode from orphan list. In
this case ext4_orphand_del() is responsible for checking whether
journal is enabled.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux