From: dmonakhov@openvz.org Subject: Re: [PATCH,RFC] Adding quotacheck functionality to e2fsck Date: Mon, 29 Mar 2010 11:50:40 +0400 Message-ID: <87hbnzlg6n.fsf@openvz.org> References: <20100326004738.GJ3145@quack.suse.cz> <20100326033824.GC21658@thunk.org> <9E7C0FF6-B02F-4470-B70A-4DBF5D5D6E0E@oracle.com> <87hbo37ay3.fsf@openvz.org> <20100326105733.GC3055@quack.suse.cz> <87y6hfibac.fsf@openvz.org> <87tys3hwvd.fsf@openvz.org> <87ljdblgvc.fsf@openvz.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andreas Dilger , tytso@mit.edu, linux-ext4@vger.kernel.org To: Jan Kara Return-path: Received: from mailhub.sw.ru ([195.214.232.25]:4104 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754740Ab0C2Hux (ORCPT ); Mon, 29 Mar 2010 03:50:53 -0400 In-Reply-To: <87ljdblgvc.fsf@openvz.org> (Jan Kara's message of "Mon, 29 Mar 2010 11:35:51 +0400") Sender: linux-ext4-owner@vger.kernel.org List-ID: Jan Kara writes: OOps.. Sorry. previous email was from me(Dmitry Monakhov) my email scrip goes crazy. Again sorry. > Dmitry Monakhov writes: > >> Dmitry Monakhov writes: >> >>> Jan Kara 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html