linux-ext4 - Re: [PATCH 0/3] ext4: don't use quota reservation for speculative metadata blocks

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, 12 Apr 2010 15:22:14 +0200
From:	Jan Kara <jack@...e.cz>
To:	Dmitry Monakhov <dmonakhov@...nvz.org>
Cc:	Eric Sandeen <sandeen@...hat.com>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 0/3] ext4: don't use quota reservation for speculative
	metadata blocks

> > Eric Sandeen <sandeen@...hat.com> writes:
> > 
> > > Dmitry Monakhov wrote:
> > >> Eric Sandeen <sandeen@...hat.com> writes:
> > >> 
> > >>> Because we can badly over-reserve metadata when we
> > >>> calculate worst-case, it complicates things for quota, since
> > >>> we must reserve and then claim later, retry on EDQUOT, etc.
> > >>> Quota is also a generally smaller pool than fs free blocks,
> > >>> so this over-reservation hurts more, and more often.
> > >>>
> > >>> I'm of the opinion that it's not the worst thing to allow
> > >>> metadata to push a user slightly over quota.  This simplifies
> > >>> the code and avoids the false quota rejections that result
> > >>> from worst-case speculation.
> > >> Hm.. Totally agree with issue description. And seem there is no another
> > >> solution except yours.
> > >> ASAIU alloc_nofail is called from places where it is impossible to fail
> > >> an allocation even if something goes wrong.
> > >> I ask because currently i'm working on EIO handling in alloc/free calls.
> > >> I've found that it is useless to fail claim/free procedures because
> > >> caller is unable to handle it properly.
> > >> It is impossible to fail following operation
> > >> ->writepage
> > >>  ->dquot_claim_space (what to do if EIO happens?)
> > >
> > > Hm, if these start returning EIO then maybe my patch should be modified
> > > to treat EDQUOT differently than EIO ... assuming callers can handle
> > > the return at all.
> > >
> > > In other words, make NOFAIL really just mean "don't fail for EDQUOT"
> > Yes. agree So we have two types of errors
> > 1) expected errors: EDQUOT
> > 2) fatal errors: (EIO/ENOSPC/ENOMEM)
> > So we need two types of flags:
> > 1)FORCE (IMHO it is better name than you proposed) to allow exceed a
> >   quota limit
> > 2)NOFAIL to allow ignore fatal errors.
> > 
> > We still need NOFAIL, because for example if something is happens in
> > ->write_page()
> >  ->dquot_claim()
> >      update_quota() -> EIO  /* update disk quota */
> >      update_bytes() /* update i_bytes count */
> > It is obvious that write_page should fail because it is too late to
> > return the error to userspace, so data will probably lost which
> > is much more dramatic bug than quota inconsistency.
> > So the only options we have is to:
> > 1) Do not modify inode->i_bytes and return error which caller will
> >    probably ignore. IMHO this is not good because result in
> >    incorrect stat()
> > 
> > 2) do as much as we can (as it happens for now), modify inode->i_bytes
> >    and return positive error code to caller.(which signal what error
> >    result in quota inconsystency only)
>   Yes, agreed that 2) is a better solution.
> 
> > This fatal errors handling logic i'll post on top of your patch-set.
> > But please change flag name from NOFAIL to FORCE.
>   Hmm, do we really need to distinguish between your NOFAIL and FORCE?
> I mean there are places where we can handle quota failures (both EDQUOT
> or others) and places where we cannot and then we just want to go on as
> seamlessly as possible. So NOFAIL flag seems to be enough...
>   Now I agree that in theory there can be some caller which might wish
> to seamlessly continue on EDQUOT and bail out on EIO but I'm not aware
> of such callsite currently so there's no immediate need for the flag.
> So Eric's patches seem to be fine to me as they are. What do you think?
  Maybe one more point - the callsite in ext4 that uses _nofail variant
really needs full NOFAIL semantics because we have no way of handling
an error there.

								Honza
-- 
Jan Kara <jack@...e.cz>
SuSE CR Labs
--
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