[PATCH v3 11/39] fs: quota: replace opened calling of ->sync_fs with sync_filesystem

Dongsheng Yang yangds.fnst at cn.fujitsu.com
Thu Sep 17 22:49:25 PDT 2015


On 09/17/2015 07:05 PM, Jan Kara wrote:
> On Thu 17-09-15 14:28:26, Dongsheng Yang wrote:
>> On 09/16/2015 06:14 PM, Jan Kara wrote:
>>> On Tue 15-09-15 17:02:06, Dongsheng Yang wrote:
>>>> There are only two places in whole kernel to call ->sync_fs directly. It
>>>> will sync fs even in read-only mode. It's not a good idea and some filesystem
>>>> would warn out if you are syncing in read-only mode. But sync_filesystem()
>>>> does filter this case out. Let's call sync_filesystem() here instead.
>>>>
>>>> Signed-off-by: Dongsheng Yang <yangds.fnst at cn.fujitsu.com>
>>>
>>> So I'd prefer ubifs used hidden system inodes for quota files, set
>>> DQUOT_QUOTA_SYS_FILE flag and so this code calling sync_fs() could be
>>> completely avoided.
>>
>> Hmmm, I think it's a good idea to make quota files SYS_FILEs. But how
>> about quota-tools? E.g, if quotacheck do some modification
>> on quota files, we would not read them without a sync_fs().
>>
>> Could you help explain how quota works in this case?
>
> So tools like quota(1) or setquota(1) work using quotactl so they don't
> need access to quota files. When quota files are system files, quotacheck
> functionality belongs into the fsck - so fsck.ubifs is responsible for
> checking consistency of quota files. How e.g. e2fsck does it is that when
> scanning inodes, it computes usage for each user / group, loads limits
> information from old quota files and then just creates new quota files with
> updated information if there's any difference to the old quota files.

About quotacheck, I think just call fsync() in it sounds good to me.
Maybe something like the attachment.

OKEY, but I found repquota is still using read() to access quota files.
Please consider that case: 1.we read quota file and there is a pagecache
for it. 2. Then kernel did some modification on quota files. But
if the files is SYS_FILES marked, dquot_quota_sync() would not drop
the pagecache, then 3. repquota would get an outdated data from pagecache.

I am not sure why ext4 works well in this case. There must be something
I am missing.

Maybe we can introduce a Q_GETBLK for quotactl() to make all
quota tools getting informations from ioctl.
>
> Another advantage of the checking functionality being in fsck is that
> fs-specific fsck can gather usage information much more efficiently (and
> fsck has to do full fs scan anyway) and there's no need to propagate quota
> usage information to userspace using FIQSIZE ioctl() and similar stuff...

So, let me try to summary the my opinions about it.
Pros:
	(1). Security. quota files shouldn't be accessible to usespace.
	(2). Efficiency. No need for quotacheck, just do it in fsck.
	(3). No need FIOQSIZE any more.
Cons:
	(1). Incompatibility:
		If I set DQUOT_QUOTA_SYS_FILE currently, there are at
least two commands would not work: quotacheck and repquota. Actually
that means the whole quota is not usable to user.

So, I think the compatibility is important to me. Then what about
setting quota files as regular files at first. After all tools (quota
tools, quotacheck, repquota, fsck) prepared, setting the
DQUOT_QUOTA_SYS_FILE seems better.

Yang
>
> 								Honza
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-quotacheck-sync-files-in-quotacheck.patch
Type: text/x-patch
Size: 1100 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20150918/b0961871/attachment.bin>


More information about the linux-mtd mailing list