linux-ext4 - Re: [PATCH,RFC] Adding quotacheck functionality to e2fsck

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 29 Mar 2010 11:50:40 +0400
From:	dmonakhov@...nvz.org
To:	Jan Kara <jack@...e.cz>
Cc:	Andreas Dilger <andreas.dilger@...cle.com>, tytso@....edu,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH,RFC] Adding quotacheck functionality to e2fsck

Jan Kara <jack@...e.cz> writes:

OOps.. Sorry. previous email was from me(Dmitry Monakhov)
my email scrip goes crazy. Again sorry.
> Dmitry Monakhov <dmonakhov@...nvz.org> writes:
>
>> Dmitry Monakhov <dmonakhov@...nvz.org> writes:
>>
>>> Jan Kara <jack@...e.cz> writes:
>>>
>>>> On Fri 26-03-10 11:18:28, Dmitry Monakhov wrote:
>>>>> > If there isn't a reason to continue using unjournaled quota (i.e. it
>>>>> > doesn't break to just move to journaled quota everywhere), then these
>>>>> > could just become aliases for the journaled quota implementation.  The
>>>>> > other alternative is to deprecate these options in the next kernel and
>>>>> > have it print out a warning on the console to tell the user to switch
>>>>> > over to the journaled version.
>>>>> The only reason to not use journalled quota by default is the currently
>>>>> it is a bit slower than unjournalled variant.
>>>>> This is because each quota change result in synchronous quotafile 
>>>>> update in per-sb-page-cache. And this update is protected by i_mutex.
>>>>> and dqio_mutex. It may be fixed easily. I've sent a RFC patch two
>>>>> month ago. I'll update it and will submit it this weekend. 
>>>>   Well, there is also some overhead caused by more IO we have to do for
>>>> quota journaling and that is essentially unavoidable. But still I believe
>>>> we should transition people to journaled quotas...
>>> Agree. IO overhead due to journalled quota is almost invisible.
>>> And it must be enabled by default after most annoying lock contention
>>> will be resolved.
>>>
>>> BTW. i've had bad news. Seems what journalled was broken recently.
>>> Right after i've wrote the first letter. i've started to update the
>>> quota-speedup patch. And during testing phase i've found that
>>> journalled quota is inconsistent after power-failure(w/o my patches).
>>> I've tested ext4.git/for-next branch
>>> Currently i'm investing the issue.
>> Ok, i've found the root of issue. dquot_transfer() wasn't called for
>> symlinks on chown due to lack of ->setattr operation.
>> Before 'dquot: cleanup dquot transfer routine' patch quota_transfer()
>> was performed by notify_transfer() itself.
> Forgot to mention that it is not journalled quota issue. But just a
> generic quota regression.
>> Now it must be handled by in corresponding ->setattr
>>
>> BTW i'm wondering, even if we don't care about quota. Inode's attributes
>> are metadata and must goes trough journal(i.e via extXXX_setattr).
>> so every inode type must has corresponding ->setattr.
> As is is always happens. Each modification result in unexpected regressions.
> In case of quota cleanup patch-set movement of quota-transfer from
> generic-setattr to fs-speciffic ->setattr result in hidden regression
> because not all inode types has correct ->setattr methods.
> Where are too many filesystems to look-at. Let's add a some
> sanity check in to notify_changes(), and remove it after 2/3 moths.
>
> Some thing like this:
> static int quota_check(struct inode *inode, struct iattr *attr)
> {
>         if (!sb_any_quota_active(inode->i_sb))
>                 return 0;
>         if (((attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) ||
>                 (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid) ||
>                 (attr->ia_valid & ATTR_SIZE)) && !inode->i_op->setattr)
>                 {
>                 WARN_ON(1);
>                 return 1;
>         }
>         return 0;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html