Thanks for your quick response! On 11/13 2013 05:03 PM, Dave Chinner wrote: > On Tue, Nov 12, 2013 at 05:29:15PM +0800, Jeff Liu wrote: >> Hi Folks, >> >> We have a user report about skip quota check on first mount/boot several >> monthes ago, the original discussion thread can be found at: >> http://oss.sgi.com/archives/xfs/2013-06/msg00170.html. >> >> As per Dave's suggestion, it would be possible to perform quota check >> in parallel, this patch series is just trying to follow up that idea. >> >> Sorry for the too long day as I have to spent most of time dealing with >> personl things in the last few monthes, I was afraid I can not quickly >> follow up the review procedure. Now the nightmare is over, it's time to >> revive this task. >> >> Also, my previous test results on my laptop and a poor desktop can not >> convience me that performs parallism quota check can really get benefits >> compare to the current single thread as both machines are shipped with >> slow disks, I even observed a little performance regression with millions >> of small files(e.g, 100 bytes) as quota check is IO bound, additionaly, >> it could affected by the seek time differences. Now with a Mackbook Air >> I bought recently, it can show significant difference. > > Results look good - they definitely point out that we can improve > the situation here. > >> In order to get some more reasonable results, I ask a friend helping >> run this test on a server which were shown as following. >> >> test environment >> - 16core, 25G ram, normal SATA disk, but the XFS is resides on a loop dev. > .... >> >> In this case, there is no regression although there is no noticeable >> improvements. :( > > Which is no surprise - there isn't any extra IO parallelism that can > be extracted from a single spindle.... > >> test environment >> - Macbook Air i7-4650U with SSD, 8G ram >> >> - # of file(million) default patched >> 1 real 0m6.367s real 0m1.972s >> user 0m0.008s user 0m0.000s >> sys 0m2.614s sys 0m0.008s >> >> 2 real 0m15.221s real 0m3.772s >> user 0m0.000s user 0m0.000s >> sys 0m6.269s sys 0m0.007s >> >> 5 real 0m36.036s real 0m8.902s >> user 0m0.000s user 0m0.002s >> sys 0m14.025s sys 0m0.006s > > But a SSD or large raid array does have unused IO parallelism we can > exploit. ;) > > Note that there is also the possibility of applying too much > parallelism for the underlying storage (think of a filesystem with > hundreds of AGs on a limited number of spindles) and hence causing > degradations due to seeking. Hence it might be worthwhile to limit > the number of AGs being scanned concurrently... Ok, maybe it could be a new mount option to let user decide how to deal with it in this situation, let me think it over. > >> Btw, The current implementation has a defeat considering the duplicated >> code at [patch 0/4] xfs: implement parallism quota check at mount time. >> Maybe it's better to introduce a new function xfs_bulkstat_ag() which can >> be used to bulkstat inodes per ag, hence it could shared at above patch while >> adjusting dquota usage per ag, i.e, xfs_qm_dqusage_adjust_perag(). > > Right, there are uses for AG-based parallelism of bulkstat for > userspace, so exposing single AG scans via the bulkstat ioctl is > something I've been intending to do for some time. Hence I'd much > prefer to see xfs_bulkstat_ag() to be implemented and then the > quotacheck code converted to use it rather than duplicating the > algorithm and code specifically to parallelise quotacheck. Thanks for the confirmation, this change will be reflected in the next round of post. > > I like the factoring of the bulkstat code (about time we did that), > but I think the factored functions should remain in xfs-itable.c > with the rest of the bulkstat code for now... > > Also, there's a race condition you haven't handled in the quotacheck > code: xfs_qm_quotacheck_dqadjust() can now be called concurrently on > a dquot from different threads to update the same dquot, and there's > no locking of the dquot to prevent this. Ah, will fix it, why I have not found this problem in the previous test? :-P > > As to the workqueues for threading, it seems overly complex. You > could create a permanent workqueue in xfs_init_workqueues() for > this, and you can use flush_workqueue() to execute and wait for all > the per-ag scans to complete once they have been queued. This gets > rid of all the lists and completions from the code. At that time, I thought the workqueue should be destroyed once the quota check procedure is complete as it only run once at mount time, will take care of it. Thanks, -Jeff _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
