| Message ID | 1442307754-13233-37-git-send-email-yangds.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: > We only care the size of regular file in ubifs for quota. > The reason is similar with the comment in ubifs_getattr(). > > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > fs/ubifs/dir.c | 3 +++ > fs/ubifs/file.c | 22 ++++++++++++++++++++++ > fs/ubifs/ubifs.h | 1 + > 3 files changed, 26 insertions(+) > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > index 802c6ad..0d3d6d3 100644 > --- a/fs/ubifs/dir.c > +++ b/fs/ubifs/dir.c > @@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { > #ifdef CONFIG_UBIFS_ATIME_SUPPORT > .update_time = ubifs_update_time, > #endif > +#ifdef CONFIG_QUOTA > + .get_qsize = ubifs_get_qsize, > +#endif > }; > > const struct file_operations ubifs_dir_operations = { > diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c > index b57ccf3..f1d792a 100644 > --- a/fs/ubifs/file.c > +++ b/fs/ubifs/file.c > @@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) > return 0; > } > > +/* > + * ubifs_get_qsize: get the quota size of a file > + * @inode: inode which we are going to get the qsize > + * > + * We only care the size of regular file in ubifs > + * for quota. The reason is similar with the comment > + * in ubifs_getattr(). > + */ > +ssize_t ubifs_get_qsize(struct inode *inode) > +{ > + if (S_ISREG(inode->i_mode)) > + return i_size_read(inode); > + else > + return 0; > +} > + The quota space is accounted in bytes. So why don't you store appropriate number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also have files occupying only say 100 bytes and everything works properly there so I don't see why ubifs needs to differ. Honza > const struct address_space_operations ubifs_file_address_operations = { > .readpage = ubifs_readpage, > .writepage = ubifs_writepage, > @@ -1656,6 +1672,9 @@ const struct inode_operations ubifs_file_inode_operations = { > #ifdef CONFIG_UBIFS_ATIME_SUPPORT > .update_time = ubifs_update_time, > #endif > +#ifdef CONFIG_QUOTA > + .get_qsize = ubifs_get_qsize, > +#endif > }; > > const struct inode_operations ubifs_symlink_inode_operations = { > @@ -1670,6 +1689,9 @@ const struct inode_operations ubifs_symlink_inode_operations = { > #ifdef CONFIG_UBIFS_ATIME_SUPPORT > .update_time = ubifs_update_time, > #endif > +#ifdef CONFIG_QUOTA > + .get_qsize = ubifs_get_qsize, > +#endif > }; > > const struct file_operations ubifs_file_operations = { > diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h > index 99cf10c..21b5dc0 100644 > --- a/fs/ubifs/ubifs.h > +++ b/fs/ubifs/ubifs.h > @@ -1759,6 +1759,7 @@ int ubifs_read_block(struct inode *inode, void *addr, unsigned int block, > int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync); > int ubifs_setattr(struct dentry *dentry, struct iattr *attr); > int ubifs_update_time(struct inode *inode, struct timespec *time, int flags); > +ssize_t ubifs_get_qsize(struct inode *inode); > > /* dir.c */ > struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir, > -- > 1.8.4.2 >
On 09/16/2015 06:00 PM, Jan Kara wrote: > On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: >> We only care the size of regular file in ubifs for quota. >> The reason is similar with the comment in ubifs_getattr(). >> >> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >> --- >> fs/ubifs/dir.c | 3 +++ >> fs/ubifs/file.c | 22 ++++++++++++++++++++++ >> fs/ubifs/ubifs.h | 1 + >> 3 files changed, 26 insertions(+) >> >> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c >> index 802c6ad..0d3d6d3 100644 >> --- a/fs/ubifs/dir.c >> +++ b/fs/ubifs/dir.c >> @@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT >> .update_time = ubifs_update_time, >> #endif >> +#ifdef CONFIG_QUOTA >> + .get_qsize = ubifs_get_qsize, >> +#endif >> }; >> >> const struct file_operations ubifs_dir_operations = { >> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c >> index b57ccf3..f1d792a 100644 >> --- a/fs/ubifs/file.c >> +++ b/fs/ubifs/file.c >> @@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) >> return 0; >> } >> >> +/* >> + * ubifs_get_qsize: get the quota size of a file >> + * @inode: inode which we are going to get the qsize >> + * >> + * We only care the size of regular file in ubifs >> + * for quota. The reason is similar with the comment >> + * in ubifs_getattr(). >> + */ >> +ssize_t ubifs_get_qsize(struct inode *inode) >> +{ >> + if (S_ISREG(inode->i_mode)) >> + return i_size_read(inode); >> + else >> + return 0; >> +} >> + > > The quota space is accounted in bytes. So why don't you store appropriate > number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also > have files occupying only say 100 bytes and everything works properly > there so I don't see why ubifs needs to differ. Ha, yes, we did not keep i_blocks in ubifs currently. Because we have no blocks in ubifs. Although we can simulate a i_block for quota, I did not do it. Let me try to show what I am thinking here. (1).Block file system are counting space with blocks. Then the quota could works in (dquot_alloc_block & i_blocks) way. I mean, account spaces by dquot_alloc_block() and FIOQSIZE can get the qsize from i_blocks. (2). But ubifs has no blocks, then I choose another way to do it, (quot_alloc_space & i_size). That means, we account quota spaces in ubifs by dquot_alloc_space() and want FIOSIZE to get i_size of inodes. Then there is no notion of *block* but only space. So, I want to make FIOSIZE more flexible here to introduce a get_qsize() into inode_operations. Yang > > Honza > >> const struct address_space_operations ubifs_file_address_operations = { >> .readpage = ubifs_readpage, >> .writepage = ubifs_writepage, >> @@ -1656,6 +1672,9 @@ const struct inode_operations ubifs_file_inode_operations = { >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT >> .update_time = ubifs_update_time, >> #endif >> +#ifdef CONFIG_QUOTA >> + .get_qsize = ubifs_get_qsize, >> +#endif >> }; >> >> const struct inode_operations ubifs_symlink_inode_operations = { >> @@ -1670,6 +1689,9 @@ const struct inode_operations ubifs_symlink_inode_operations = { >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT >> .update_time = ubifs_update_time, >> #endif >> +#ifdef CONFIG_QUOTA >> + .get_qsize = ubifs_get_qsize, >> +#endif >> }; >> >> const struct file_operations ubifs_file_operations = { >> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h >> index 99cf10c..21b5dc0 100644 >> --- a/fs/ubifs/ubifs.h >> +++ b/fs/ubifs/ubifs.h >> @@ -1759,6 +1759,7 @@ int ubifs_read_block(struct inode *inode, void *addr, unsigned int block, >> int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync); >> int ubifs_setattr(struct dentry *dentry, struct iattr *attr); >> int ubifs_update_time(struct inode *inode, struct timespec *time, int flags); >> +ssize_t ubifs_get_qsize(struct inode *inode); >> >> /* dir.c */ >> struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir, >> -- >> 1.8.4.2 >> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu 17-09-15 15:23:11, Dongsheng Yang wrote: > On 09/16/2015 06:00 PM, Jan Kara wrote: > >On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: > >>We only care the size of regular file in ubifs for quota. > >>The reason is similar with the comment in ubifs_getattr(). > >> > >>Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > >>--- > >> fs/ubifs/dir.c | 3 +++ > >> fs/ubifs/file.c | 22 ++++++++++++++++++++++ > >> fs/ubifs/ubifs.h | 1 + > >> 3 files changed, 26 insertions(+) > >> > >>diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > >>index 802c6ad..0d3d6d3 100644 > >>--- a/fs/ubifs/dir.c > >>+++ b/fs/ubifs/dir.c > >>@@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { > >> #ifdef CONFIG_UBIFS_ATIME_SUPPORT > >> .update_time = ubifs_update_time, > >> #endif > >>+#ifdef CONFIG_QUOTA > >>+ .get_qsize = ubifs_get_qsize, > >>+#endif > >> }; > >> > >> const struct file_operations ubifs_dir_operations = { > >>diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c > >>index b57ccf3..f1d792a 100644 > >>--- a/fs/ubifs/file.c > >>+++ b/fs/ubifs/file.c > >>@@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) > >> return 0; > >> } > >> > >>+/* > >>+ * ubifs_get_qsize: get the quota size of a file > >>+ * @inode: inode which we are going to get the qsize > >>+ * > >>+ * We only care the size of regular file in ubifs > >>+ * for quota. The reason is similar with the comment > >>+ * in ubifs_getattr(). > >>+ */ > >>+ssize_t ubifs_get_qsize(struct inode *inode) > >>+{ > >>+ if (S_ISREG(inode->i_mode)) > >>+ return i_size_read(inode); > >>+ else > >>+ return 0; > >>+} > >>+ > > > >The quota space is accounted in bytes. So why don't you store appropriate > >number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also > >have files occupying only say 100 bytes and everything works properly > >there so I don't see why ubifs needs to differ. > > Ha, yes, we did not keep i_blocks in ubifs currently. Because we have > no blocks in ubifs. Although we can simulate a i_block for quota, I > did not do it. Let me try to show what I am thinking here. > > (1).Block file system are counting space with blocks. Then the quota > could works in (dquot_alloc_block & i_blocks) way. I mean, account > spaces by dquot_alloc_block() and FIOQSIZE can get the qsize from > i_blocks. > > (2). But ubifs has no blocks, then I choose another way to do it, > (quot_alloc_space & i_size). That means, we account quota spaces > in ubifs by dquot_alloc_space() and want FIOSIZE to get i_size > of inodes. Then there is no notion of *block* but only space. > > So, I want to make FIOSIZE more flexible here to introduce a get_qsize() > into inode_operations. But when you use dquot_alloc_space(), then i_blocks / i_bytes will be updated accordingly by quota code (after all, those values are used when inode gets transferred between users on chown to update quota information accordingly). So I don't see a reason why you should need to use i_size in FIOSIZE. As a side note, I think that using inode size as the amount of space used is wrong. I believe even ubifs has a notion of how many bytes of storage the file occupies and *this* information should be fed into quota subsystem and updated via dquot_alloc_space(). That way it will be more or less consistent with how quota for other filesystems works as well. However this is more or less filesystem internal decision so I can live with ubifs doing it either way... Honza
On 09/17/2015 08:00 PM, Jan Kara wrote: > On Thu 17-09-15 15:23:11, Dongsheng Yang wrote: >> On 09/16/2015 06:00 PM, Jan Kara wrote: >>> On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: >>>> We only care the size of regular file in ubifs for quota. >>>> The reason is similar with the comment in ubifs_getattr(). >>>> >>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >>>> --- >>>> fs/ubifs/dir.c | 3 +++ >>>> fs/ubifs/file.c | 22 ++++++++++++++++++++++ >>>> fs/ubifs/ubifs.h | 1 + >>>> 3 files changed, 26 insertions(+) >>>> >>>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c >>>> index 802c6ad..0d3d6d3 100644 >>>> --- a/fs/ubifs/dir.c >>>> +++ b/fs/ubifs/dir.c >>>> @@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { >>>> #ifdef CONFIG_UBIFS_ATIME_SUPPORT >>>> .update_time = ubifs_update_time, >>>> #endif >>>> +#ifdef CONFIG_QUOTA >>>> + .get_qsize = ubifs_get_qsize, >>>> +#endif >>>> }; >>>> >>>> const struct file_operations ubifs_dir_operations = { >>>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c >>>> index b57ccf3..f1d792a 100644 >>>> --- a/fs/ubifs/file.c >>>> +++ b/fs/ubifs/file.c >>>> @@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) >>>> return 0; >>>> } >>>> >>>> +/* >>>> + * ubifs_get_qsize: get the quota size of a file >>>> + * @inode: inode which we are going to get the qsize >>>> + * >>>> + * We only care the size of regular file in ubifs >>>> + * for quota. The reason is similar with the comment >>>> + * in ubifs_getattr(). >>>> + */ >>>> +ssize_t ubifs_get_qsize(struct inode *inode) >>>> +{ >>>> + if (S_ISREG(inode->i_mode)) >>>> + return i_size_read(inode); >>>> + else >>>> + return 0; >>>> +} >>>> + >>> >>> The quota space is accounted in bytes. So why don't you store appropriate >>> number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also >>> have files occupying only say 100 bytes and everything works properly >>> there so I don't see why ubifs needs to differ. >> >> Ha, yes, we did not keep i_blocks in ubifs currently. Because we have >> no blocks in ubifs. Although we can simulate a i_block for quota, I >> did not do it. Let me try to show what I am thinking here. >> >> (1).Block file system are counting space with blocks. Then the quota >> could works in (dquot_alloc_block & i_blocks) way. I mean, account >> spaces by dquot_alloc_block() and FIOQSIZE can get the qsize from >> i_blocks. >> >> (2). But ubifs has no blocks, then I choose another way to do it, >> (quot_alloc_space & i_size). That means, we account quota spaces >> in ubifs by dquot_alloc_space() and want FIOSIZE to get i_size >> of inodes. Then there is no notion of *block* but only space. >> >> So, I want to make FIOSIZE more flexible here to introduce a get_qsize() >> into inode_operations. > > But when you use dquot_alloc_space(), then i_blocks / i_bytes will be > updated accordingly by quota code (after all, those values are used when > inode gets transferred between users on chown to update quota information > accordingly). So I don't see a reason why you should need to use i_size in > FIOSIZE. Yes, dquot_alloc_space() would update i_blocks. But ubifs_iget() would never set i_blocks. So i_blocks would be 0 if you get inode from flash again. Yes, I can make ubifs to maintain a i_blocks. But..... > > As a side note, I think that using inode size as the amount of space used > is wrong. I believe even ubifs has a notion of how many bytes of storage > the file occupies and *this* information should be fed into quota subsystem > and updated via dquot_alloc_space(). That way it will be more or less > consistent with how quota for other filesystems works as well. .... TBH, there is a little different with other filesystems. I did not use the "disk" space, but the file space in ubifs quota, although dquot means disk quota. Same with btrfs quota. If we use disk space for quota, the most common problem from user is that: why I did not reach the limit but I can not write any byte. COW in btrfs or out-place-updating in ubifs makes this problem much worse. So I choose file space here for ubifs. Yang > However this is more or less filesystem internal decision so I can live with ubifs doing > it either way... > > Honza > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 18-09-15 14:14:48, Dongsheng Yang wrote: > On 09/17/2015 08:00 PM, Jan Kara wrote: > >On Thu 17-09-15 15:23:11, Dongsheng Yang wrote: > >>On 09/16/2015 06:00 PM, Jan Kara wrote: > >>>On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: > >>>>We only care the size of regular file in ubifs for quota. > >>>>The reason is similar with the comment in ubifs_getattr(). > >>>> > >>>>Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > >>>>--- > >>>> fs/ubifs/dir.c | 3 +++ > >>>> fs/ubifs/file.c | 22 ++++++++++++++++++++++ > >>>> fs/ubifs/ubifs.h | 1 + > >>>> 3 files changed, 26 insertions(+) > >>>> > >>>>diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c > >>>>index 802c6ad..0d3d6d3 100644 > >>>>--- a/fs/ubifs/dir.c > >>>>+++ b/fs/ubifs/dir.c > >>>>@@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { > >>>> #ifdef CONFIG_UBIFS_ATIME_SUPPORT > >>>> .update_time = ubifs_update_time, > >>>> #endif > >>>>+#ifdef CONFIG_QUOTA > >>>>+ .get_qsize = ubifs_get_qsize, > >>>>+#endif > >>>> }; > >>>> > >>>> const struct file_operations ubifs_dir_operations = { > >>>>diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c > >>>>index b57ccf3..f1d792a 100644 > >>>>--- a/fs/ubifs/file.c > >>>>+++ b/fs/ubifs/file.c > >>>>@@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) > >>>> return 0; > >>>> } > >>>> > >>>>+/* > >>>>+ * ubifs_get_qsize: get the quota size of a file > >>>>+ * @inode: inode which we are going to get the qsize > >>>>+ * > >>>>+ * We only care the size of regular file in ubifs > >>>>+ * for quota. The reason is similar with the comment > >>>>+ * in ubifs_getattr(). > >>>>+ */ > >>>>+ssize_t ubifs_get_qsize(struct inode *inode) > >>>>+{ > >>>>+ if (S_ISREG(inode->i_mode)) > >>>>+ return i_size_read(inode); > >>>>+ else > >>>>+ return 0; > >>>>+} > >>>>+ > >>> > >>>The quota space is accounted in bytes. So why don't you store appropriate > >>>number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also > >>>have files occupying only say 100 bytes and everything works properly > >>>there so I don't see why ubifs needs to differ. > >> > >>Ha, yes, we did not keep i_blocks in ubifs currently. Because we have > >>no blocks in ubifs. Although we can simulate a i_block for quota, I > >>did not do it. Let me try to show what I am thinking here. > >> > >>(1).Block file system are counting space with blocks. Then the quota > >>could works in (dquot_alloc_block & i_blocks) way. I mean, account > >>spaces by dquot_alloc_block() and FIOQSIZE can get the qsize from > >>i_blocks. > >> > >>(2). But ubifs has no blocks, then I choose another way to do it, > >>(quot_alloc_space & i_size). That means, we account quota spaces > >>in ubifs by dquot_alloc_space() and want FIOSIZE to get i_size > >>of inodes. Then there is no notion of *block* but only space. > >> > >>So, I want to make FIOSIZE more flexible here to introduce a get_qsize() > >>into inode_operations. > > > >But when you use dquot_alloc_space(), then i_blocks / i_bytes will be > >updated accordingly by quota code (after all, those values are used when > >inode gets transferred between users on chown to update quota information > >accordingly). So I don't see a reason why you should need to use i_size in > >FIOSIZE. > > Yes, dquot_alloc_space() would update i_blocks. But ubifs_iget() would > never set i_blocks. So i_blocks would be 0 if you get inode from flash > again. Yes, I can make ubifs to maintain a i_blocks. But..... > > > >As a side note, I think that using inode size as the amount of space used > >is wrong. I believe even ubifs has a notion of how many bytes of storage > >the file occupies and *this* information should be fed into quota subsystem > >and updated via dquot_alloc_space(). That way it will be more or less > >consistent with how quota for other filesystems works as well. > > .... TBH, there is a little different with other filesystems. I did not > use the "disk" space, but the file space in ubifs quota, although dquot > means disk quota. Same with btrfs quota. If we use disk space for quota, > the most common problem from user is that: why I did not reach the limit > but I can not write any byte. COW in btrfs or out-place-updating in > ubifs makes this problem much worse. > > So I choose file space here for ubifs. OK, so these are really two separate questions. I understand your choice of using file space as the amount of space to account for quota purposes and I'm fine with that choice. Another thing is that regardless of how you decide to do quota accounting, you must maintain i_blocks / i_bytes to contain proper value because dquot_transfer() uses that information to update quota usage when inode owner is changed. Honza
On 09/18/2015 07:20 PM, Jan Kara wrote: > On Fri 18-09-15 14:14:48, Dongsheng Yang wrote: >> On 09/17/2015 08:00 PM, Jan Kara wrote: >>> On Thu 17-09-15 15:23:11, Dongsheng Yang wrote: >>>> On 09/16/2015 06:00 PM, Jan Kara wrote: >>>>> On Tue 15-09-15 17:02:31, Dongsheng Yang wrote: >>>>>> We only care the size of regular file in ubifs for quota. >>>>>> The reason is similar with the comment in ubifs_getattr(). >>>>>> >>>>>> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >>>>>> --- >>>>>> fs/ubifs/dir.c | 3 +++ >>>>>> fs/ubifs/file.c | 22 ++++++++++++++++++++++ >>>>>> fs/ubifs/ubifs.h | 1 + >>>>>> 3 files changed, 26 insertions(+) >>>>>> >>>>>> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c >>>>>> index 802c6ad..0d3d6d3 100644 >>>>>> --- a/fs/ubifs/dir.c >>>>>> +++ b/fs/ubifs/dir.c >>>>>> @@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { >>>>>> #ifdef CONFIG_UBIFS_ATIME_SUPPORT >>>>>> .update_time = ubifs_update_time, >>>>>> #endif >>>>>> +#ifdef CONFIG_QUOTA >>>>>> + .get_qsize = ubifs_get_qsize, >>>>>> +#endif >>>>>> }; >>>>>> >>>>>> const struct file_operations ubifs_dir_operations = { >>>>>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c >>>>>> index b57ccf3..f1d792a 100644 >>>>>> --- a/fs/ubifs/file.c >>>>>> +++ b/fs/ubifs/file.c >>>>>> @@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +/* >>>>>> + * ubifs_get_qsize: get the quota size of a file >>>>>> + * @inode: inode which we are going to get the qsize >>>>>> + * >>>>>> + * We only care the size of regular file in ubifs >>>>>> + * for quota. The reason is similar with the comment >>>>>> + * in ubifs_getattr(). >>>>>> + */ >>>>>> +ssize_t ubifs_get_qsize(struct inode *inode) >>>>>> +{ >>>>>> + if (S_ISREG(inode->i_mode)) >>>>>> + return i_size_read(inode); >>>>>> + else >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>> >>>>> The quota space is accounted in bytes. So why don't you store appropriate >>>>> number of bytes the file consumes in i_blocks / i_bytes? Reiserfs can also >>>>> have files occupying only say 100 bytes and everything works properly >>>>> there so I don't see why ubifs needs to differ. >>>> >>>> Ha, yes, we did not keep i_blocks in ubifs currently. Because we have >>>> no blocks in ubifs. Although we can simulate a i_block for quota, I >>>> did not do it. Let me try to show what I am thinking here. >>>> >>>> (1).Block file system are counting space with blocks. Then the quota >>>> could works in (dquot_alloc_block & i_blocks) way. I mean, account >>>> spaces by dquot_alloc_block() and FIOQSIZE can get the qsize from >>>> i_blocks. >>>> >>>> (2). But ubifs has no blocks, then I choose another way to do it, >>>> (quot_alloc_space & i_size). That means, we account quota spaces >>>> in ubifs by dquot_alloc_space() and want FIOSIZE to get i_size >>>> of inodes. Then there is no notion of *block* but only space. >>>> >>>> So, I want to make FIOSIZE more flexible here to introduce a get_qsize() >>>> into inode_operations. >>> >>> But when you use dquot_alloc_space(), then i_blocks / i_bytes will be >>> updated accordingly by quota code (after all, those values are used when >>> inode gets transferred between users on chown to update quota information >>> accordingly). So I don't see a reason why you should need to use i_size in >>> FIOSIZE. >> >> Yes, dquot_alloc_space() would update i_blocks. But ubifs_iget() would >> never set i_blocks. So i_blocks would be 0 if you get inode from flash >> again. Yes, I can make ubifs to maintain a i_blocks. But..... >>> >>> As a side note, I think that using inode size as the amount of space used >>> is wrong. I believe even ubifs has a notion of how many bytes of storage >>> the file occupies and *this* information should be fed into quota subsystem >>> and updated via dquot_alloc_space(). That way it will be more or less >>> consistent with how quota for other filesystems works as well. >> >> .... TBH, there is a little different with other filesystems. I did not >> use the "disk" space, but the file space in ubifs quota, although dquot >> means disk quota. Same with btrfs quota. If we use disk space for quota, >> the most common problem from user is that: why I did not reach the limit >> but I can not write any byte. COW in btrfs or out-place-updating in >> ubifs makes this problem much worse. >> >> So I choose file space here for ubifs. > > OK, so these are really two separate questions. I understand your choice of > using file space as the amount of space to account for quota purposes and > I'm fine with that choice. Another thing is that regardless of how you > decide to do quota accounting, you must maintain i_blocks / i_bytes to > contain proper value because dquot_transfer() uses that information to update > quota usage when inode owner is changed. But if we don't use i_blocks to get qsize, what we care only in dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks is not correct in dquot_transfer() in ubifs, that's okey, because we will never use this value, right? Yang > > Honza > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: > On 09/18/2015 07:20 PM, Jan Kara wrote: > >>.... TBH, there is a little different with other filesystems. I did not > >>use the "disk" space, but the file space in ubifs quota, although dquot > >>means disk quota. Same with btrfs quota. If we use disk space for quota, > >>the most common problem from user is that: why I did not reach the limit > >>but I can not write any byte. COW in btrfs or out-place-updating in > >>ubifs makes this problem much worse. > >> > >>So I choose file space here for ubifs. > > > >OK, so these are really two separate questions. I understand your choice of > >using file space as the amount of space to account for quota purposes and > >I'm fine with that choice. Another thing is that regardless of how you > >decide to do quota accounting, you must maintain i_blocks / i_bytes to > >contain proper value because dquot_transfer() uses that information to update > >quota usage when inode owner is changed. > > But if we don't use i_blocks to get qsize, what we care only in > dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks > is not correct in dquot_transfer() in ubifs, that's okey, because we > will never use this value, right? dquot_transfer() will use the value - when file F changes owner from user A to user B, then you need to decrement amount of space used by F from A's quota usage and add that amount to B's quota usage. And the amount of space is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See __dquot_transfer() in fs/quota/dquot.c for details. Honza
On 09/21/2015 05:13 PM, Jan Kara wrote: > On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: >> On 09/18/2015 07:20 PM, Jan Kara wrote: >>>> .... TBH, there is a little different with other filesystems. I did not >>>> use the "disk" space, but the file space in ubifs quota, although dquot >>>> means disk quota. Same with btrfs quota. If we use disk space for quota, >>>> the most common problem from user is that: why I did not reach the limit >>>> but I can not write any byte. COW in btrfs or out-place-updating in >>>> ubifs makes this problem much worse. >>>> >>>> So I choose file space here for ubifs. >>> >>> OK, so these are really two separate questions. I understand your choice of >>> using file space as the amount of space to account for quota purposes and >>> I'm fine with that choice. Another thing is that regardless of how you >>> decide to do quota accounting, you must maintain i_blocks / i_bytes to >>> contain proper value because dquot_transfer() uses that information to update >>> quota usage when inode owner is changed. >> >> But if we don't use i_blocks to get qsize, what we care only in >> dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks >> is not correct in dquot_transfer() in ubifs, that's okey, because we >> will never use this value, right? > > dquot_transfer() will use the value - when file F changes owner from user A > to user B, then you need to decrement amount of space used by F from A's > quota usage and add that amount to B's quota usage. And the amount of space > is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See > __dquot_transfer() in fs/quota/dquot.c for details. Yes, I see it. But if ubifs doesn't use i_blocks and i_bytes to stand for quota size, as I mentioned I want to use i_size. Then we will never use i_blocks. So we only need a to transfer dquot->dq_dqb values. That means, even the i_blocks in dquot_transfer() is not correct, we don't care it. we only need to make sure values in dquot_dq_dqb are transfered, such as dquot->dq_dqb->curspace for user B is equal with i_size. Yang > > Honza > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 21-09-15 17:16:49, Dongsheng Yang wrote: > On 09/21/2015 05:13 PM, Jan Kara wrote: > >On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: > >>On 09/18/2015 07:20 PM, Jan Kara wrote: > >>>>.... TBH, there is a little different with other filesystems. I did not > >>>>use the "disk" space, but the file space in ubifs quota, although dquot > >>>>means disk quota. Same with btrfs quota. If we use disk space for quota, > >>>>the most common problem from user is that: why I did not reach the limit > >>>>but I can not write any byte. COW in btrfs or out-place-updating in > >>>>ubifs makes this problem much worse. > >>>> > >>>>So I choose file space here for ubifs. > >>> > >>>OK, so these are really two separate questions. I understand your choice of > >>>using file space as the amount of space to account for quota purposes and > >>>I'm fine with that choice. Another thing is that regardless of how you > >>>decide to do quota accounting, you must maintain i_blocks / i_bytes to > >>>contain proper value because dquot_transfer() uses that information to update > >>>quota usage when inode owner is changed. > >> > >>But if we don't use i_blocks to get qsize, what we care only in > >>dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks > >>is not correct in dquot_transfer() in ubifs, that's okey, because we > >>will never use this value, right? > > > >dquot_transfer() will use the value - when file F changes owner from user A > >to user B, then you need to decrement amount of space used by F from A's > >quota usage and add that amount to B's quota usage. And the amount of space > >is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See > >__dquot_transfer() in fs/quota/dquot.c for details. > > Yes, I see it. But if ubifs doesn't use i_blocks and i_bytes to stand > for quota size, as I mentioned I want to use i_size. Then we will never > use i_blocks. So we only need a to transfer dquot->dq_dqb values. That > means, even the i_blocks in dquot_transfer() is not correct, we don't > care it. we only need to make sure values in dquot_dq_dqb are > transfered, such as dquot->dq_dqb->curspace for user B is equal with > i_size. I think you are missing one thing: Assume user A has files F1 (1MB) F2 (2MB) F3 (3MB). User B has one file G1 (4MB). So user A uses 6 MB in total (stored in dquot for user A in dqb_curspace field). User B uses 4MB. Now you do "chown B F1" You need to change dq_dqb of F1 to point to user B quota structure as you explain, that is correct. However you also need to subtract 1MB from user A's total usage (which is stored in struct dquot for user A) since file F1 no longer belongs to him. And you need to add 1 MB to user B's total usage. After this user A will have total usage of 5 MB and user B has usage of 5 MB as well. You need to properly update the total usage of each user so that you can check the usage against quota limits. Honza
On 09/21/2015 05:44 PM, Jan Kara wrote: > On Mon 21-09-15 17:16:49, Dongsheng Yang wrote: >> On 09/21/2015 05:13 PM, Jan Kara wrote: >>> On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: >>>> On 09/18/2015 07:20 PM, Jan Kara wrote: >>>>>> .... TBH, there is a little different with other filesystems. I did not >>>>>> use the "disk" space, but the file space in ubifs quota, although dquot >>>>>> means disk quota. Same with btrfs quota. If we use disk space for quota, >>>>>> the most common problem from user is that: why I did not reach the limit >>>>>> but I can not write any byte. COW in btrfs or out-place-updating in >>>>>> ubifs makes this problem much worse. >>>>>> >>>>>> So I choose file space here for ubifs. >>>>> >>>>> OK, so these are really two separate questions. I understand your choice of >>>>> using file space as the amount of space to account for quota purposes and >>>>> I'm fine with that choice. Another thing is that regardless of how you >>>>> decide to do quota accounting, you must maintain i_blocks / i_bytes to >>>>> contain proper value because dquot_transfer() uses that information to update >>>>> quota usage when inode owner is changed. >>>> >>>> But if we don't use i_blocks to get qsize, what we care only in >>>> dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks >>>> is not correct in dquot_transfer() in ubifs, that's okey, because we >>>> will never use this value, right? >>> >>> dquot_transfer() will use the value - when file F changes owner from user A >>> to user B, then you need to decrement amount of space used by F from A's >>> quota usage and add that amount to B's quota usage. And the amount of space >>> is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See >>> __dquot_transfer() in fs/quota/dquot.c for details. >> >> Yes, I see it. But if ubifs doesn't use i_blocks and i_bytes to stand >> for quota size, as I mentioned I want to use i_size. Then we will never >> use i_blocks. So we only need a to transfer dquot->dq_dqb values. That >> means, even the i_blocks in dquot_transfer() is not correct, we don't >> care it. we only need to make sure values in dquot_dq_dqb are >> transfered, such as dquot->dq_dqb->curspace for user B is equal with >> i_size. > > I think you are missing one thing: > > Assume user A has files F1 (1MB) F2 (2MB) F3 (3MB). > User B has one file G1 (4MB). > > So user A uses 6 MB in total (stored in dquot for user A in dqb_curspace > field). User B uses 4MB. > > Now you do "chown B F1" > > You need to change dq_dqb of F1 to point to user B quota structure as you > explain, that is correct. However you also need to subtract 1MB from user > A's total usage (which is stored in struct dquot for user A) since file F1 > no longer belongs to him. And you need to add 1 MB to user B's total usage. > > After this user A will have total usage of 5 MB and user B has usage of 5 > MB as well. You need to properly update the total usage of each user so > that you can check the usage against quota limits. But that's all done in dquot_transfer(). It dquot_decr_space(from) and dquot_incr_space(to). Let me show the test result in my implementation. (1). root -> 1M/2M/3M yds ->4M [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ total 11M -rw-r--r--. 1 root root 1.0M Sep 21 15:54 1M -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ *** Report for user quotas on device /dev/ubi0_0 Block grace time: 7days; Inode grace time: 7days Block limits File limits User used soft hard grace used soft hard grace ---------------------------------------------------------------------- root -- 6144 0 0 4 0 0 yds -- 4096 0 0 1 0 0 (2). chown yds /mnt/ubifs/1M [root@atest-guest linux_compile]# chown yds /mnt/ubifs/1M [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ total 11M -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ *** Report for user quotas on device /dev/ubi0_0 Block grace time: 7days; Inode grace time: 7days Block limits File limits User used soft hard grace used soft hard grace ---------------------------------------------------------------------- root -- 5120 0 0 3 0 0 yds -- 5120 0 0 2 0 0 (3). quotacheck and check again. [root@atest-guest linux_compile]# quotaoff /mnt/ubifs && quotacheck -u /mnt/ubifs/ [root@atest-guest linux_compile]# ll -h /mnt/ubifs total 11M -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group -rw-------. 1 root root 7.0K Sep 21 16:00 aquota.user [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ *** Report for user quotas on device /dev/ubi0_0 Block grace time: 7days; Inode grace time: 7days Block limits File limits User used soft hard grace used soft hard grace ---------------------------------------------------------------------- root -- 5120 0 0 3 0 0 yds -- 5120 0 0 2 0 0 Yang > > Honza > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 21-09-15 19:02:30, Dongsheng Yang wrote: > On 09/21/2015 05:44 PM, Jan Kara wrote: > >On Mon 21-09-15 17:16:49, Dongsheng Yang wrote: > >>On 09/21/2015 05:13 PM, Jan Kara wrote: > >>>On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: > >>>>On 09/18/2015 07:20 PM, Jan Kara wrote: > >>>>>>.... TBH, there is a little different with other filesystems. I did not > >>>>>>use the "disk" space, but the file space in ubifs quota, although dquot > >>>>>>means disk quota. Same with btrfs quota. If we use disk space for quota, > >>>>>>the most common problem from user is that: why I did not reach the limit > >>>>>>but I can not write any byte. COW in btrfs or out-place-updating in > >>>>>>ubifs makes this problem much worse. > >>>>>> > >>>>>>So I choose file space here for ubifs. > >>>>> > >>>>>OK, so these are really two separate questions. I understand your choice of > >>>>>using file space as the amount of space to account for quota purposes and > >>>>>I'm fine with that choice. Another thing is that regardless of how you > >>>>>decide to do quota accounting, you must maintain i_blocks / i_bytes to > >>>>>contain proper value because dquot_transfer() uses that information to update > >>>>>quota usage when inode owner is changed. > >>>> > >>>>But if we don't use i_blocks to get qsize, what we care only in > >>>>dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks > >>>>is not correct in dquot_transfer() in ubifs, that's okey, because we > >>>>will never use this value, right? > >>> > >>>dquot_transfer() will use the value - when file F changes owner from user A > >>>to user B, then you need to decrement amount of space used by F from A's > >>>quota usage and add that amount to B's quota usage. And the amount of space > >>>is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See > >>>__dquot_transfer() in fs/quota/dquot.c for details. > >> > >>Yes, I see it. But if ubifs doesn't use i_blocks and i_bytes to stand > >>for quota size, as I mentioned I want to use i_size. Then we will never > >>use i_blocks. So we only need a to transfer dquot->dq_dqb values. That > >>means, even the i_blocks in dquot_transfer() is not correct, we don't > >>care it. we only need to make sure values in dquot_dq_dqb are > >>transfered, such as dquot->dq_dqb->curspace for user B is equal with > >>i_size. > > > >I think you are missing one thing: > > > >Assume user A has files F1 (1MB) F2 (2MB) F3 (3MB). > >User B has one file G1 (4MB). > > > >So user A uses 6 MB in total (stored in dquot for user A in dqb_curspace > >field). User B uses 4MB. > > > >Now you do "chown B F1" > > > >You need to change dq_dqb of F1 to point to user B quota structure as you > >explain, that is correct. However you also need to subtract 1MB from user > >A's total usage (which is stored in struct dquot for user A) since file F1 > >no longer belongs to him. And you need to add 1 MB to user B's total usage. > > > >After this user A will have total usage of 5 MB and user B has usage of 5 > >MB as well. You need to properly update the total usage of each user so > >that you can check the usage against quota limits. > > But that's all done in dquot_transfer(). It dquot_decr_space(from) and > dquot_incr_space(to). Let me show the test result in my implementation. Yes, dquot_decr_space(transfer_from[cnt], cur_space) and dquot_incr_space(transfer_to[cnt], cur_space) are the functions changing usage. However have a look at what cur_space is. It is set as: cur_space = inode_get_bytes(inode); and inode_get_bytes() is: ret = (((loff_t)inode->i_blocks) << 9) + inode->i_bytes; So for dquot_transfer() to work correctly, filesystem has to properly maintain i_blocks and i_bytes. > (1). root -> 1M/2M/3M yds ->4M > > [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ > total 11M > -rw-r--r--. 1 root root 1.0M Sep 21 15:54 1M > -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M > -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M > -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M > -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group > -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user > [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ > *** Report for user quotas on device /dev/ubi0_0 > Block grace time: 7days; Inode grace time: 7days > Block limits File limits > User used soft hard grace used soft hard grace > ---------------------------------------------------------------------- > root -- 6144 0 0 4 0 0 > yds -- 4096 0 0 1 0 0 > > > (2). chown yds /mnt/ubifs/1M > [root@atest-guest linux_compile]# chown yds /mnt/ubifs/1M > [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ > total 11M > -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M > -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M > -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M > -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M > -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group > -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user > [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ > *** Report for user quotas on device /dev/ubi0_0 > Block grace time: 7days; Inode grace time: 7days > Block limits File limits > User used soft hard grace used soft hard grace > ---------------------------------------------------------------------- > root -- 5120 0 0 3 0 0 > yds -- 5120 0 0 2 0 0 > > > (3). quotacheck and check again. > [root@atest-guest linux_compile]# quotaoff /mnt/ubifs && quotacheck > -u /mnt/ubifs/ > [root@atest-guest linux_compile]# ll -h /mnt/ubifs > total 11M > -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M > -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M > -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M > -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M > -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group > -rw-------. 1 root root 7.0K Sep 21 16:00 aquota.user > [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ > *** Report for user quotas on device /dev/ubi0_0 > Block grace time: 7days; Inode grace time: 7days > Block limits File limits > User used soft hard grace used soft hard grace > ---------------------------------------------------------------------- > root -- 5120 0 0 3 0 0 > yds -- 5120 0 0 2 0 0 Yes, this case is working because quota code updated i_blocks properly when creating files. But if you do not set i_blocks when reading inodes (which seems to be the case) I expect the following would not work: mount -t ubifs -o usrjquota=aquota.user,jqfmt=vfsv0 <dev> /mnt quotacheck -vu /mnt quotaon -vu /mnt dd if=/dev/zero of=file1 bs=1M count=1 # Umount fs to prune cached information umount /mnt mount -t ubifs -o usrjquota=aquota.user,jqfmt=vfsv0 <dev> /mnt quotaon -vu /mnt chown yds /mnt/file1 repquota -u /mnt/ Now unless you set i_blocks when reading inode, yds would not be accounted for 1 MB /mnt/file1 is using... Honza
On 09/23/2015 03:42 PM, Jan Kara wrote: > On Mon 21-09-15 19:02:30, Dongsheng Yang wrote: >> On 09/21/2015 05:44 PM, Jan Kara wrote: >>> On Mon 21-09-15 17:16:49, Dongsheng Yang wrote: >>>> On 09/21/2015 05:13 PM, Jan Kara wrote: >>>>> On Mon 21-09-15 12:35:01, Dongsheng Yang wrote: >>>>>> On 09/18/2015 07:20 PM, Jan Kara wrote: >>>>>>>> .... TBH, there is a little different with other filesystems. I did not >>>>>>>> use the "disk" space, but the file space in ubifs quota, although dquot >>>>>>>> means disk quota. Same with btrfs quota. If we use disk space for quota, >>>>>>>> the most common problem from user is that: why I did not reach the limit >>>>>>>> but I can not write any byte. COW in btrfs or out-place-updating in >>>>>>>> ubifs makes this problem much worse. >>>>>>>> >>>>>>>> So I choose file space here for ubifs. >>>>>>> >>>>>>> OK, so these are really two separate questions. I understand your choice of >>>>>>> using file space as the amount of space to account for quota purposes and >>>>>>> I'm fine with that choice. Another thing is that regardless of how you >>>>>>> decide to do quota accounting, you must maintain i_blocks / i_bytes to >>>>>>> contain proper value because dquot_transfer() uses that information to update >>>>>>> quota usage when inode owner is changed. >>>>>> >>>>>> But if we don't use i_blocks to get qsize, what we care only in >>>>>> dquot_transter() is dquot->dq_dqb. That means, even if the i_blocks >>>>>> is not correct in dquot_transfer() in ubifs, that's okey, because we >>>>>> will never use this value, right? >>>>> >>>>> dquot_transfer() will use the value - when file F changes owner from user A >>>>> to user B, then you need to decrement amount of space used by F from A's >>>>> quota usage and add that amount to B's quota usage. And the amount of space >>>>> is obtained via inode_get_bytes() which uses i_blocks and i_bytes. See >>>>> __dquot_transfer() in fs/quota/dquot.c for details. >>>> >>>> Yes, I see it. But if ubifs doesn't use i_blocks and i_bytes to stand >>>> for quota size, as I mentioned I want to use i_size. Then we will never >>>> use i_blocks. So we only need a to transfer dquot->dq_dqb values. That >>>> means, even the i_blocks in dquot_transfer() is not correct, we don't >>>> care it. we only need to make sure values in dquot_dq_dqb are >>>> transfered, such as dquot->dq_dqb->curspace for user B is equal with >>>> i_size. >>> >>> I think you are missing one thing: >>> >>> Assume user A has files F1 (1MB) F2 (2MB) F3 (3MB). >>> User B has one file G1 (4MB). >>> >>> So user A uses 6 MB in total (stored in dquot for user A in dqb_curspace >>> field). User B uses 4MB. >>> >>> Now you do "chown B F1" >>> >>> You need to change dq_dqb of F1 to point to user B quota structure as you >>> explain, that is correct. However you also need to subtract 1MB from user >>> A's total usage (which is stored in struct dquot for user A) since file F1 >>> no longer belongs to him. And you need to add 1 MB to user B's total usage. >>> >>> After this user A will have total usage of 5 MB and user B has usage of 5 >>> MB as well. You need to properly update the total usage of each user so >>> that you can check the usage against quota limits. >> >> But that's all done in dquot_transfer(). It dquot_decr_space(from) and >> dquot_incr_space(to). Let me show the test result in my implementation. > > Yes, dquot_decr_space(transfer_from[cnt], cur_space) and > dquot_incr_space(transfer_to[cnt], cur_space) are the functions changing > usage. However have a look at what cur_space is. It is set as: > > cur_space = inode_get_bytes(inode); > > and inode_get_bytes() is: > > ret = (((loff_t)inode->i_blocks) << 9) + inode->i_bytes; > > So for dquot_transfer() to work correctly, filesystem has to properly > maintain i_blocks and i_bytes. Oh, my mistake. you are right. We have to maintain i_blocks for quota then. will update it. Yang > >> (1). root -> 1M/2M/3M yds ->4M >> >> [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ >> total 11M >> -rw-r--r--. 1 root root 1.0M Sep 21 15:54 1M >> -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M >> -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M >> -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M >> -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group >> -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user >> [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ >> *** Report for user quotas on device /dev/ubi0_0 >> Block grace time: 7days; Inode grace time: 7days >> Block limits File limits >> User used soft hard grace used soft hard grace >> ---------------------------------------------------------------------- >> root -- 6144 0 0 4 0 0 >> yds -- 4096 0 0 1 0 0 >> >> >> (2). chown yds /mnt/ubifs/1M >> [root@atest-guest linux_compile]# chown yds /mnt/ubifs/1M >> [root@atest-guest linux_compile]# ll -h /mnt/ubifs/ >> total 11M >> -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M >> -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M >> -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M >> -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M >> -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group >> -rw-------. 1 root root 7.0K Sep 21 15:53 aquota.user >> [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ >> *** Report for user quotas on device /dev/ubi0_0 >> Block grace time: 7days; Inode grace time: 7days >> Block limits File limits >> User used soft hard grace used soft hard grace >> ---------------------------------------------------------------------- >> root -- 5120 0 0 3 0 0 >> yds -- 5120 0 0 2 0 0 >> >> >> (3). quotacheck and check again. >> [root@atest-guest linux_compile]# quotaoff /mnt/ubifs && quotacheck >> -u /mnt/ubifs/ >> [root@atest-guest linux_compile]# ll -h /mnt/ubifs >> total 11M >> -rw-r--r--. 1 yds root 1.0M Sep 21 15:54 1M >> -rw-r--r--. 1 root root 2.0M Sep 21 15:54 2M >> -rw-r--r--. 1 root root 3.0M Sep 21 15:54 3M >> -rw-r--r--. 1 yds root 4.0M Sep 21 15:55 4M >> -rw-------. 1 root root 6.0K Sep 21 15:53 aquota.group >> -rw-------. 1 root root 7.0K Sep 21 16:00 aquota.user >> [root@atest-guest linux_compile]# repquota -u /mnt/ubifs/ >> *** Report for user quotas on device /dev/ubi0_0 >> Block grace time: 7days; Inode grace time: 7days >> Block limits File limits >> User used soft hard grace used soft hard grace >> ---------------------------------------------------------------------- >> root -- 5120 0 0 3 0 0 >> yds -- 5120 0 0 2 0 0 > > Yes, this case is working because quota code updated i_blocks properly when > creating files. But if you do not set i_blocks when reading inodes > (which seems to be the case) I expect the following would not work: > > mount -t ubifs -o usrjquota=aquota.user,jqfmt=vfsv0 <dev> /mnt > quotacheck -vu /mnt > quotaon -vu /mnt > dd if=/dev/zero of=file1 bs=1M count=1 > # Umount fs to prune cached information > umount /mnt > mount -t ubifs -o usrjquota=aquota.user,jqfmt=vfsv0 <dev> /mnt > quotaon -vu /mnt > chown yds /mnt/file1 > repquota -u /mnt/ > > Now unless you set i_blocks when reading inode, yds would not be accounted > for 1 MB /mnt/file1 is using... > > Honza > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 802c6ad..0d3d6d3 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1205,6 +1205,9 @@ const struct inode_operations ubifs_dir_inode_operations = { #ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, #endif +#ifdef CONFIG_QUOTA + .get_qsize = ubifs_get_qsize, +#endif }; const struct file_operations ubifs_dir_operations = { diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index b57ccf3..f1d792a 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1636,6 +1636,22 @@ static int ubifs_file_mmap(struct file *file, struct vm_area_struct *vma) return 0; } +/* + * ubifs_get_qsize: get the quota size of a file + * @inode: inode which we are going to get the qsize + * + * We only care the size of regular file in ubifs + * for quota. The reason is similar with the comment + * in ubifs_getattr(). + */ +ssize_t ubifs_get_qsize(struct inode *inode) +{ + if (S_ISREG(inode->i_mode)) + return i_size_read(inode); + else + return 0; +} + const struct address_space_operations ubifs_file_address_operations = { .readpage = ubifs_readpage, .writepage = ubifs_writepage, @@ -1656,6 +1672,9 @@ const struct inode_operations ubifs_file_inode_operations = { #ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, #endif +#ifdef CONFIG_QUOTA + .get_qsize = ubifs_get_qsize, +#endif }; const struct inode_operations ubifs_symlink_inode_operations = { @@ -1670,6 +1689,9 @@ const struct inode_operations ubifs_symlink_inode_operations = { #ifdef CONFIG_UBIFS_ATIME_SUPPORT .update_time = ubifs_update_time, #endif +#ifdef CONFIG_QUOTA + .get_qsize = ubifs_get_qsize, +#endif }; const struct file_operations ubifs_file_operations = { diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 99cf10c..21b5dc0 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1759,6 +1759,7 @@ int ubifs_read_block(struct inode *inode, void *addr, unsigned int block, int ubifs_fsync(struct file *file, loff_t start, loff_t end, int datasync); int ubifs_setattr(struct dentry *dentry, struct iattr *attr); int ubifs_update_time(struct inode *inode, struct timespec *time, int flags); +ssize_t ubifs_get_qsize(struct inode *inode); /* dir.c */ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
We only care the size of regular file in ubifs for quota. The reason is similar with the comment in ubifs_getattr(). Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/ubifs/dir.c | 3 +++ fs/ubifs/file.c | 22 ++++++++++++++++++++++ fs/ubifs/ubifs.h | 1 + 3 files changed, 26 insertions(+)