On Tue, May 26, 2020 at 04:11:50PM +0200, Otto Moerbeek wrote:
> On Tue, May 26, 2020 at 03:54:15PM +0200, Otto Moerbeek wrote:
>
> > On Tue, May 26, 2020 at 07:51:28AM -0600, Todd C. Miller wrote:
> >
> > > On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote:
> > >
> > > > Apart from the noting the strange Subject: I also like to mention one
> > > > change in the way cylinder groups are scanned. The current code scans
> > > > forward and backward, which causes an uneven distribution of full cgs
> > > > (the upper end of the cgs will get full first). Fix that by always
> > > > scanning forward, wrapping to cg 0 if needed.
> > >
> > > Should that be a separate commit? I can't find any problems
> > > with the diff but I haven't tried running with it yet.
> > >
> > > - todd
> >
> > Yeah, I can do that. Note that it must be comitted first, since the
> > loop condition is always true if I change the loop var to unsigned.
> >
> > -Otto
> >
>
> And a new diff. I accidentally capitalized a letter just before sending.
> Thanks to naddy for spotting that.
Here's the separate diff for the prefcg loops. From FreeBSD.
OK?
-Otto
Index: ffs_alloc.c
===================================================================
RCS file: /cvs/src/sys/ufs/ffs/ffs_alloc.c,v
retrieving revision 1.111
diff -u -p -r1.111 ffs_alloc.c
--- ffs_alloc.c 28 May 2020 15:48:29 -0000 1.111
+++ ffs_alloc.c 28 May 2020 18:47:53 -0000
@@ -515,7 +518,7 @@ ffs_dirpref(struct inode *pip)
maxcontigdirs = 1;
/*
- * Limit number of dirs in one cg and reserve space for
+ * Limit number of dirs in one cg and reserve space for
* regular files, but only if we have no deficit in
* inodes or space.
*
@@ -524,16 +527,17 @@ ffs_dirpref(struct inode *pip)
* We scan from our preferred cylinder group forward looking
* for a cylinder group that meets our criterion. If we get
* to the final cylinder group and do not find anything,
- * we start scanning backwards from our preferred cylinder
- * group. The ideal would be to alternate looking forward
- * and backward, but tha tis just too complex to code for
- * the gain it would get. The most likely place where the
- * backward scan would take effect is when we start near
- * the end of the filesystem and do not find anything from
- * where we are to the end. In that case, scanning backward
- * will likely find us a suitable cylinder group much closer
- * to our desired location than if we were to start scanning
- * forward from the beginning for the filesystem.
+ * we start scanning forwards from the beginning of the
+ * filesystem. While it might seem sensible to start scanning
+ * backwards or even to alternate looking forward and backward,
+ * this approach fails badly when the filesystem is nearly full.
+ * Specifically, we first search all the areas that have no space
+ * and finally try the one preceding that. We repeat this on
+ * every request and in the case of the final block end up
+ * searching the entire filesystem. By jumping to the front
+ * of the filesystem, our future forward searches always look
+ * in new cylinder groups so finds every possible block after
+ * one pass over the filesystem.
*/
for (cg = prefcg; cg < fs->fs_ncg; cg++)
if (fs->fs_cs(fs, cg).cs_ndir < maxndir &&
@@ -542,7 +546,7 @@ ffs_dirpref(struct inode *pip)
if (fs->fs_contigdirs[cg] < maxcontigdirs)
goto end;
}
- for (cg = prefcg - 1; cg >= 0; cg--)
+ for (cg = 0; cg < prefcg; cg++)
if (fs->fs_cs(fs, cg).cs_ndir < maxndir &&
fs->fs_cs(fs, cg).cs_nifree >= minifree &&
fs->fs_cs(fs, cg).cs_nbfree >= minbfree) {
@@ -555,7 +559,7 @@ ffs_dirpref(struct inode *pip)
for (cg = prefcg; cg < fs->fs_ncg; cg++)
if (fs->fs_cs(fs, cg).cs_nifree >= avgifree)
goto end;
- for (cg = prefcg - 1; cg >= 0; cg--)
+ for (cg = 0; cg < prefcg; cg++)
if (fs->fs_cs(fs, cg).cs_nifree >= avgifree)
goto end;
end:
