> > Eric Sandeen <sandeen@xxxxxxxxxx> writes: > > > > > Dmitry Monakhov wrote: > > >> Eric Sandeen <sandeen@xxxxxxxxxx> 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@xxxxxxx> SuSE CR Labs -- 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
