On 2010-11-03, at 03:31, Dmitry wrote:
> On Wed, 3 Nov 2010 03:06:41 -0600, Andreas Dilger <adilger@xxxxxxxxx> wrote:
>>
>> Basically, the first hunk is moving the ext4_handle_valid() into the caller, and the second is avoiding a call to ext4_orphan_del->ext4_handle_valid(), which would likely be an unmeasurable performance difference. No locks are taken (or avoided) before ext4_handle_valid() is checked.
>
> But in case of nojournal mode it will be called with noop result.
> but orphan_del(NULL, inode) will result in useless locking.
Looking at ext4_orphan_del(), I'm not sure why it checks:
/* ext4_handle_valid() assumes a valid handle_t pointer */
if (handle && !ext4_handle_valid(handle))
return 0;
when in ext4_orphan_add() it is only checks:
if (!ext4_handle_valid(handle))
return 0;
I think the extra check for "handle" in ext4_orphan_del() is not needed, despite the comment there, because none of the other callsites for ext4_handle_valid() do this extra check.
>> Even better would be a no-lock check of list_empty(&ei->i_orphan()) before getting s_orphan_lock, since it shouldn't be possible to have two threads adding/removing the same inode to the orphan list (otherwise the inode may not be on the orphan list at all since add/del is not refcounted). The only reason for s_orphan_lock is to prevent corruption of the global list.
>
> Yes, this is the best solution. Actually i've thought about that, but
> end up with more obvious fix for nojournal mode.
> With that we can avoid 1 of 3 locks on truncate in journal mode.
> I'll test that solution.
Cheers, Andreas
--
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
