| Message ID | 1438235311-23788-21-git-send-email-yangds.fnst@cn.fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: > We have to implement 3 functions to support quota in ubifs: > ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas() > > ubifs_quota_read(): > callback to read quota file. > ubifs_quota_write(): > callback to write data to quota file. > ubifs_get_quotas(): > callback to get quota instance for inode. > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 219 insertions(+) > > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c > index bc57685..3609d7b 100644 > --- a/fs/ubifs/super.c > +++ b/fs/ubifs/super.c > @@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c) > return 0; > } > > +#ifdef CONFIG_QUOTA > +static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data, > + size_t len, loff_t off) > +{ > + struct inode *inode = sb_dqopt(sb)->files[type]; > + unsigned long block = off >> UBIFS_BLOCK_SHIFT; > + int offset = off & (sb->s_blocksize - 1); > + int tocopy = 0; > + size_t toread; > + char *block_buf; > + struct ubifs_data_node *dn; > + int ret, err = 0; > + > + toread = len; Minor nit, please group same types together and also group assignments as other UBI/FS codes does. > + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); > + if (!dn) { > + err = -ENOMEM; > + goto out; > + } > + > + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); > + if (!block_buf) { > + err = -ENOMEM; > + goto free_dn; > + } > + > + if (offset) { > + /* Read the un-aligned data in first block */ > + tocopy = sb->s_blocksize - offset; > + if (toread < tocopy) > + tocopy = toread; min()? > + ret = ubifs_read_block(inode, block_buf, block, dn); > + if (ret) { > + if (ret != -ENOENT) { if (ret && ret != -ENOENT)? > + err = ret; > + goto free_buf; > + } > + } > + > + memcpy(data, block_buf + offset, tocopy); > + > + block++; > + toread -= tocopy; > + data += tocopy; > + tocopy = 0; > + } > + > + while (toread > 0) { > + tocopy = sb->s_blocksize < toread ? > + sb->s_blocksize : toread; min()? > + > + /* Break to read the last block */ > + if (tocopy < sb->s_blocksize) > + break; Hmm, can't you combine this check with the above condition? > + ret = ubifs_read_block(inode, data, block, dn); > + if (ret) { > + if (ret != -ENOENT) { See above. > + err = ret; > + goto free_buf; > + } > + } > + block++;; superfluous semicolon. > + toread -= tocopy; > + data += tocopy; > + tocopy = 0; > + } > + > + if (tocopy) { > + /* Read the data in last block */ > + ret = ubifs_read_block(inode, block_buf, block, dn); > + if (ret) { > + if (ret != -ENOENT) { See above. Maybe the -ENOENT stuff can be integrated into ubifs_read_block(). All callers deal with it the same way... > + err = ret; > + goto free_buf; > + } > + } > + > + memcpy(data, block_buf, tocopy); > + } > +free_buf: > + kfree(block_buf); > +free_dn: > + kfree(dn); > +out: > + if (!err) > + return len; > + return err; > +} > + > +static ssize_t ubifs_quota_write(struct super_block *sb, int type, > + const char *data, size_t len, loff_t off) > +{ > + struct inode *inode = sb_dqopt(sb)->files[type]; > + unsigned long block = off >> UBIFS_BLOCK_SHIFT; > + struct ubifs_info *c = inode->i_sb->s_fs_info; > + int offset = off & (sb->s_blocksize - 1); > + union ubifs_key key; > + int tocopy = 0; > + size_t towrite = len; > + int ret, err = 0; > + struct ubifs_budget_req req = {}; > + int new_block = 0; > + struct ubifs_data_node *dn; > + char *block_buf; See above. > + /* There are dirtied blocks and new blockes, but we call them all > + * new blocks here, the budgeting is same for them. > + */ > + new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize); > + if (offset) > + new_block += 1; > + > + req.new_block_num = new_block; > + > + err = ubifs_budget_space(c, &req); > + if (err) > + goto out; > + > + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); > + if (!dn) { > + err = -ENOMEM; > + goto release_budget; > + } > + > + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); > + if (!block_buf) { > + err = -ENOMEM; > + goto free_dn; > + } > + > + if (offset) { > + /* Write the un-aligned data in first block */ > + tocopy = sb->s_blocksize - offset; > + if (towrite < tocopy) > + tocopy = towrite; See above. > + ret = ubifs_read_block(inode, block_buf, block, dn); > + if (ret) { > + if (ret != -ENOENT) { > + err = ret; > + goto free_buf; > + } > + memset(block_buf, 0, sb->s_blocksize); > + } > + > + memcpy(block_buf + offset, data, tocopy); > + > + data_key_init(c, &key, inode->i_ino, block); > + err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize); Hmm, I'm confused. You write the block containing the quota information to an inode's data? How can be determined whether data is real data and what is quota data? This clearly shows that I know not much about the quota subsystem. 8^) > + if (err) { > + goto free_buf; > + } > + block++; > + towrite -= tocopy; > + data += tocopy; > + tocopy = 0; > + } > + > + while (towrite > 0) { > + tocopy = sb->s_blocksize < towrite ? > + sb->s_blocksize : towrite; > + > + /* Break to read the last block */ > + if (tocopy < sb->s_blocksize) > + break; See above. Maybe you can combine all this common code. > + data_key_init(c, &key, inode->i_ino, block); > + err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize); > + if (err) > + goto free_buf; > + block++;; See above. I'll review your other patches these days. For now I'm too sleepy. Thanks, //richard
On 08/04/2015 05:46 AM, Richard Weinberger wrote: > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: >> We have to implement 3 functions to support quota in ubifs: >> ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas() >> [...] > > I'll review your other patches these days. > For now I'm too sleepy. Thank you so much, Richard. :) I will reply all the comments from you and Jan when you finish the review. Thanx Yang > > Thanks, > //richard > . >
On 08/04/2015 05:46 AM, Richard Weinberger wrote: > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: >> We have to implement 3 functions to support quota in ubifs: >> ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas() >> >> ubifs_quota_read(): >> callback to read quota file. >> ubifs_quota_write(): >> callback to write data to quota file. >> ubifs_get_quotas(): >> callback to get quota instance for inode. >> >> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >> --- >> fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 219 insertions(+) >> >> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c >> index bc57685..3609d7b 100644 >> --- a/fs/ubifs/super.c >> +++ b/fs/ubifs/super.c >> @@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c) >> return 0; >> } >> >> +#ifdef CONFIG_QUOTA >> +static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data, >> + size_t len, loff_t off) >> +{ >> + struct inode *inode = sb_dqopt(sb)->files[type]; >> + unsigned long block = off >> UBIFS_BLOCK_SHIFT; >> + int offset = off & (sb->s_blocksize - 1); >> + int tocopy = 0; >> + size_t toread; >> + char *block_buf; >> + struct ubifs_data_node *dn; >> + int ret, err = 0; >> + >> + toread = len; > > Minor nit, please group same types together and also group assignments > as other UBI/FS codes does. Hm, okey. Good suggestion. > >> + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); >> + if (!dn) { >> + err = -ENOMEM; >> + goto out; >> + } >> + >> + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); >> + if (!block_buf) { >> + err = -ENOMEM; >> + goto free_dn; >> + } >> + >> + if (offset) { >> + /* Read the un-aligned data in first block */ >> + tocopy = sb->s_blocksize - offset; >> + if (toread < tocopy) >> + tocopy = toread; > > min()? okey, thanx > >> + ret = ubifs_read_block(inode, block_buf, block, dn); >> + if (ret) { >> + if (ret != -ENOENT) { > > if (ret && ret != -ENOENT)? Yea, agree. > >> + err = ret; >> + goto free_buf; >> + } >> + } >> + >> + memcpy(data, block_buf + offset, tocopy); >> + >> + block++; >> + toread -= tocopy; >> + data += tocopy; >> + tocopy = 0; >> + } >> + >> + while (toread > 0) { >> + tocopy = sb->s_blocksize < toread ? >> + sb->s_blocksize : toread; > > min()? Yes, correct. > >> + >> + /* Break to read the last block */ >> + if (tocopy < sb->s_blocksize) >> + break; > > Hmm, can't you combine this check with the above condition? Sure. > >> + ret = ubifs_read_block(inode, data, block, dn); >> + if (ret) { >> + if (ret != -ENOENT) { > > See above. Thanx > >> + err = ret; >> + goto free_buf; >> + } >> + } >> + block++;; > > superfluous semicolon. Oh, right you are. > >> + toread -= tocopy; >> + data += tocopy; >> + tocopy = 0; >> + } >> + >> + if (tocopy) { >> + /* Read the data in last block */ >> + ret = ubifs_read_block(inode, block_buf, block, dn); >> + if (ret) { >> + if (ret != -ENOENT) { > > See above. Maybe the -ENOENT stuff can be integrated into ubifs_read_block(). > All callers deal with it the same way... Good point. Will try it. > >> + err = ret; >> + goto free_buf; >> + } >> + } >> + >> + memcpy(data, block_buf, tocopy); >> + } >> +free_buf: >> + kfree(block_buf); >> +free_dn: >> + kfree(dn); >> +out: >> + if (!err) >> + return len; >> + return err; >> +} >> + >> +static ssize_t ubifs_quota_write(struct super_block *sb, int type, >> + const char *data, size_t len, loff_t off) >> +{ >> + struct inode *inode = sb_dqopt(sb)->files[type]; >> + unsigned long block = off >> UBIFS_BLOCK_SHIFT; >> + struct ubifs_info *c = inode->i_sb->s_fs_info; >> + int offset = off & (sb->s_blocksize - 1); >> + union ubifs_key key; >> + int tocopy = 0; >> + size_t towrite = len; >> + int ret, err = 0; >> + struct ubifs_budget_req req = {}; >> + int new_block = 0; >> + struct ubifs_data_node *dn; >> + char *block_buf; > > See above. > >> + /* There are dirtied blocks and new blockes, but we call them all >> + * new blocks here, the budgeting is same for them. >> + */ >> + new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize); >> + if (offset) >> + new_block += 1; >> + >> + req.new_block_num = new_block; >> + >> + err = ubifs_budget_space(c, &req); >> + if (err) >> + goto out; >> + >> + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); >> + if (!dn) { >> + err = -ENOMEM; >> + goto release_budget; >> + } >> + >> + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); >> + if (!block_buf) { >> + err = -ENOMEM; >> + goto free_dn; >> + } >> + >> + if (offset) { >> + /* Write the un-aligned data in first block */ >> + tocopy = sb->s_blocksize - offset; >> + if (towrite < tocopy) >> + tocopy = towrite; > > See above. > >> + ret = ubifs_read_block(inode, block_buf, block, dn); >> + if (ret) { >> + if (ret != -ENOENT) { >> + err = ret; >> + goto free_buf; >> + } >> + memset(block_buf, 0, sb->s_blocksize); >> + } >> + >> + memcpy(block_buf + offset, data, tocopy); >> + >> + data_key_init(c, &key, inode->i_ino, block); >> + err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize); > > Hmm, I'm confused. You write the block containing the quota information to an inode's data? > How can be determined whether data is real data and what is quota data? Ha, so we need a special writing function for quota file, xxxfs_quota_write(). This function would bypass the quota recording, so quota data would not be contained in by itself. > This clearly shows that I know not much about the quota subsystem. 8^) > >> + if (err) { >> + goto free_buf; >> + } >> + block++; >> + towrite -= tocopy; >> + data += tocopy; >> + tocopy = 0; >> + } >> + >> + while (towrite > 0) { >> + tocopy = sb->s_blocksize < towrite ? >> + sb->s_blocksize : towrite; >> + >> + /* Break to read the last block */ >> + if (tocopy < sb->s_blocksize) >> + break; > > See above. Maybe you can combine all this common code. Agree, thanx > >> + data_key_init(c, &key, inode->i_ino, block); >> + err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize); >> + if (err) >> + goto free_buf; >> + block++;; > > See above. > > I'll review your other patches these days. > For now I'm too sleepy. Thanx a lot Richard. Your review is always helpful to me. Yang > > Thanks, > //richard > . >
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index bc57685..3609d7b 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -940,6 +940,220 @@ static int check_volume_empty(struct ubifs_info *c) return 0; } +#ifdef CONFIG_QUOTA +static ssize_t ubifs_quota_read(struct super_block *sb, int type, char *data, + size_t len, loff_t off) +{ + struct inode *inode = sb_dqopt(sb)->files[type]; + unsigned long block = off >> UBIFS_BLOCK_SHIFT; + int offset = off & (sb->s_blocksize - 1); + int tocopy = 0; + size_t toread; + char *block_buf; + struct ubifs_data_node *dn; + int ret, err = 0; + + toread = len; + + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); + if (!dn) { + err = -ENOMEM; + goto out; + } + + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); + if (!block_buf) { + err = -ENOMEM; + goto free_dn; + } + + if (offset) { + /* Read the un-aligned data in first block */ + tocopy = sb->s_blocksize - offset; + if (toread < tocopy) + tocopy = toread; + + ret = ubifs_read_block(inode, block_buf, block, dn); + if (ret) { + if (ret != -ENOENT) { + err = ret; + goto free_buf; + } + } + + memcpy(data, block_buf + offset, tocopy); + + block++; + toread -= tocopy; + data += tocopy; + tocopy = 0; + } + + while (toread > 0) { + tocopy = sb->s_blocksize < toread ? + sb->s_blocksize : toread; + + /* Break to read the last block */ + if (tocopy < sb->s_blocksize) + break; + + ret = ubifs_read_block(inode, data, block, dn); + if (ret) { + if (ret != -ENOENT) { + err = ret; + goto free_buf; + } + } + block++;; + toread -= tocopy; + data += tocopy; + tocopy = 0; + } + + if (tocopy) { + /* Read the data in last block */ + ret = ubifs_read_block(inode, block_buf, block, dn); + if (ret) { + if (ret != -ENOENT) { + err = ret; + goto free_buf; + } + } + + memcpy(data, block_buf, tocopy); + } +free_buf: + kfree(block_buf); +free_dn: + kfree(dn); +out: + if (!err) + return len; + return err; +} + +static ssize_t ubifs_quota_write(struct super_block *sb, int type, + const char *data, size_t len, loff_t off) +{ + struct inode *inode = sb_dqopt(sb)->files[type]; + unsigned long block = off >> UBIFS_BLOCK_SHIFT; + struct ubifs_info *c = inode->i_sb->s_fs_info; + int offset = off & (sb->s_blocksize - 1); + union ubifs_key key; + int tocopy = 0; + size_t towrite = len; + int ret, err = 0; + struct ubifs_budget_req req = {}; + int new_block = 0; + struct ubifs_data_node *dn; + char *block_buf; + + /* There are dirtied blocks and new blockes, but we call them all + * new blocks here, the budgeting is same for them. + */ + new_block = DIV_ROUND_UP(len - (sb->s_blocksize - offset), sb->s_blocksize); + if (offset) + new_block += 1; + + req.new_block_num = new_block; + + err = ubifs_budget_space(c, &req); + if (err) + goto out; + + dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS); + if (!dn) { + err = -ENOMEM; + goto release_budget; + } + + block_buf = kmalloc(sb->s_blocksize, GFP_NOFS); + if (!block_buf) { + err = -ENOMEM; + goto free_dn; + } + + if (offset) { + /* Write the un-aligned data in first block */ + tocopy = sb->s_blocksize - offset; + if (towrite < tocopy) + tocopy = towrite; + + ret = ubifs_read_block(inode, block_buf, block, dn); + if (ret) { + if (ret != -ENOENT) { + err = ret; + goto free_buf; + } + memset(block_buf, 0, sb->s_blocksize); + } + + memcpy(block_buf + offset, data, tocopy); + + data_key_init(c, &key, inode->i_ino, block); + err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize); + if (err) { + goto free_buf; + } + block++; + towrite -= tocopy; + data += tocopy; + tocopy = 0; + } + + while (towrite > 0) { + tocopy = sb->s_blocksize < towrite ? + sb->s_blocksize : towrite; + + /* Break to read the last block */ + if (tocopy < sb->s_blocksize) + break; + + data_key_init(c, &key, inode->i_ino, block); + err = ubifs_jnl_write_data(c, inode, &key, data, sb->s_blocksize); + if (err) + goto free_buf; + block++;; + towrite -= tocopy; + data += tocopy; + tocopy = 0; + } + + if (tocopy) { + /* Read the data in last block */ + ret = ubifs_read_block(inode, block_buf, block, dn); + if (ret) { + if (ret != -ENOENT) { + err = ret; + goto free_buf; + } + memset(block_buf, 0, sb->s_blocksize); + } + + memcpy(block_buf, data, tocopy); + data_key_init(c, &key, inode->i_ino, block); + err = ubifs_jnl_write_data(c, inode, &key, block_buf, sb->s_blocksize); + if (err) + goto free_buf; + } +free_buf: + kfree(block_buf); +free_dn: + kfree(dn); +release_budget: + ubifs_release_budget(c, &req); +out: + if (!err) + return len; + return err; +} + +static struct dquot **ubifs_get_dquots(struct inode *inode) +{ + return ubifs_inode(inode)->i_dquot; +} +#endif + /* * UBIFS mount options. * @@ -1939,6 +2153,11 @@ const struct super_operations ubifs_super_operations = { .remount_fs = ubifs_remount_fs, .show_options = ubifs_show_options, .sync_fs = ubifs_sync_fs, +#ifdef CONFIG_QUOTA + .quota_read = ubifs_quota_read, + .quota_write = ubifs_quota_write, + .get_dquots = ubifs_get_dquots, +#endif }; /**
We have to implement 3 functions to support quota in ubifs: ubifs_quota_read(), ubifs_quota_write() and ubifs_get_quotas() ubifs_quota_read(): callback to read quota file. ubifs_quota_write(): callback to write data to quota file. ubifs_get_quotas(): callback to get quota instance for inode. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/ubifs/super.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+)