| 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
| ||
|
Message-Id: <1231806287.6752.25.camel@mingming-laptop>
Date: Mon, 12 Jan 2009 16:24:47 -0800
From: Mingming Cao <cmm@...ibm.com>
To: Jan Kara <jack@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>, tytso <tytso@....edu>,
linux-ext4 <linux-ext4@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH V5 2/5] quota: Add quota claim and release reserved
quota blocks operations
Hi Jan,
在 2009-01-06二的 10:52 +0100，Jan Kara写道：
> Hi,
>
> On Mon 05-01-09 20:40:37, Mingming Cao wrote:
> > quota: Add quota reservation claim and released operations
> >
> > Reserved quota will be claimed at the block allocation time. Over-booked
> > quota could be returned back with the release callback function.
> >
> > Signed-off-by: Mingming Cao <cmm@...ibm.com>
> Just a few minor comments below.
>
> > ---
> > fs/dquot.c | 113 +++++++++++++++++++++++++++++++++++++++++++++--
> > include/linux/quota.h | 6 ++
> > include/linux/quotaops.h | 53 ++++++++++++++++++++++
> > 3 files changed, 168 insertions(+), 4 deletions(-)
> >
> > Index: linux-2.6.28-git7/include/linux/quota.h
> > ===================================================================
> > --- linux-2.6.28-git7.orig/include/linux/quota.h 2009-01-05 17:26:47.000000000 -0800
> > +++ linux-2.6.28-git7/include/linux/quota.h 2009-01-05 17:26:48.000000000 -0800
> > @@ -311,6 +311,12 @@ struct dquot_operations {
> > int (*write_info) (struct super_block *, int); /* Write of quota "superblock" */
> > /* reserve quota for delayed block allocation */
> > int (*reserve_space) (struct inode *, qsize_t, int);
> > + /* claim reserved quota for delayed alloc */
> > + int (*claim_space) (struct inode *, qsize_t);
> > + /* release rsved quota for delayed alloc */
> > + void (*release_rsv) (struct inode *, qsize_t);
> > + /* get reserved quota for delayed alloc */
> > + unsigned long long (*get_reserved_space) (struct inode *);
> ^^^ qsize_t would be more appropriate here IMO.
>
>
> > };
> >
> > /* Operations handling requests from userspace */
> <snip>
>
> > Index: linux-2.6.28-git7/fs/dquot.c
> > ===================================================================
> > --- linux-2.6.28-git7.orig/fs/dquot.c 2009-01-05 17:26:47.000000000 -0800
> > +++ linux-2.6.28-git7/fs/dquot.c 2009-01-05 20:06:23.000000000 -0800
> > @@ -903,6 +903,23 @@ static inline void dquot_resv_space(stru
> > dquot->dq_dqb.dqb_rsvspace += number;
> > }
> >
> > +/*
> > + * Claim reserved quota space
> > + */
> > +static void dquot_claim_reserved_space(struct dquot *dquot,
> > + qsize_t number)
> > +{
> > + WARN_ON(dquot->dq_dqb.dqb_rsvspace < number);
> > + dquot->dq_dqb.dqb_curspace += number;
> > + dquot->dq_dqb.dqb_rsvspace -= number;
> > +}
> > +
> > +static inline
> > +void dquot_free_reserved_space(struct dquot *dquot, qsize_t number)
> > +{
> > + dquot->dq_dqb.dqb_rsvspace -= number;
> > +}
> > +
> > static inline void dquot_decr_inodes(struct dquot *dquot, qsize_t number)
> > {
> > if (sb_dqopt(dquot->dq_sb)->flags & DQUOT_NEGATIVE_USAGE ||
> > @@ -1432,6 +1449,75 @@ warn_put_all:
> > return ret;
> > }
> >
> > +int dquot_claim_space(struct inode *inode, qsize_t number)
> > +{
> > + int cnt;
> > + int ret = QUOTA_OK;
> > +
> > + if (IS_NOQUOTA(inode)) {
> > + inode_add_bytes(inode, number);
> > + goto out;
> > + }
> > +
> > + down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > + if (IS_NOQUOTA(inode)) {
> > + up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > + inode_add_bytes(inode, number);
> > + goto out;
> > + }
> > +
> > + spin_lock(&dq_data_lock);
> > + /* Claim reserved quotas to allocated quotas */
> > + for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> > + if (inode->i_dquot[cnt] != NODQUOT)
> > + dquot_claim_reserved_space(inode->i_dquot[cnt],
> > + number);
> > + }
> > + /* Update inode bytes */
> > + inode_add_bytes(inode, number);
> > + spin_unlock(&dq_data_lock);
> > + /* Dirtify all the dquots - this can block when journalling */
> > + for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> > + if (inode->i_dquot[cnt])
> > + mark_dquot_dirty(inode->i_dquot[cnt]);
> > + up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +out:
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(dquot_claim_space);
> > +
> > +/*
> > + * Release reserved quota space
> > + */
> > +void dquot_release_reserved_space(struct inode *inode, qsize_t number)
> > +{
> > + int cnt;
> > + struct dquot *dquot;
> > +
> > + if (IS_NOQUOTA(inode))
> > + goto out;
> > +
> > + down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > + if (IS_NOQUOTA(inode))
> > + goto out_unlock;
> > +
> > + spin_lock(&dq_data_lock);
> > + /* Release reserved dquots */
> > + for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> > + if (inode->i_dquot[cnt] != NODQUOT) {
> > + dquot = inode->i_dquot[cnt];
> > + dquot_free_reserved_space(dquot, number);
> > + }
> > + }
> > + spin_unlock(&dq_data_lock);
> Variable dquot seems to be quite useless here...
>
Yeah, it was probably due to the rewrite of this part. I removed this
varible .
> > +
> > +out_unlock:
> > + up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +out:
> > + return;
> > +}
> > +EXPORT_SYMBOL(dquot_release_reserved_space);
> > +
> > /*
> > * This operation can block, but only after everything is updated
> > */
> > @@ -1507,6 +1593,19 @@ int dquot_free_inode(const struct inode
> > up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > return QUOTA_OK;
> > }
> Empty line here?
>
Removed.
> > +/*
> > + * call back function, get reserved quota space from underlying fs
> > + */
> > +qsize_t dquot_get_reserved_space(struct inode *inode)
> > +{
> > + qsize_t reserved_space = 0;
> > +
> > + if (sb_any_quota_active(inode->i_sb) &&
> > + inode->i_sb->dq_op->get_reserved_space)
> > + reserved_space = inode->i_sb->dq_op->get_reserved_space(inode)
> > + << inode->i_blkbits;
> > + return reserved_space;
> > +}
> The function should really return number of bytes as its name suggests.
> In principle there's no reason why inode could not have 5 bytes reserved
> :).
> Hmm, sorry for my comment to ext4 patch. That is correct given what is
> written here. I was just confused by the name of the callback.
>
No problem. I am fine with either way. To ext4, the number of bytes
reserved should be always aligned with the fs block size. But it
probably nicer to keep the interface consistant with other quota
operations.
I switched to the method you suggested ,having underlying fs return
number of bytes reserved. Please take a look, thanks.
View attachment "quota-claim-reservation-vfs.patch" of type "text/x-patch" (8533 bytes)
Powered by blists - more mailing lists