Re: [PATCH]: newfs(8) FS_OPTSPACE vs FS_OPTTIME bug

2003-01-27 Thread David Schultz
Thus spake Maxim Konovalov <[EMAIL PROTECTED]>:
> newfs(8) incorrectly claims that FS_OPTTIME is unavailable when
> minfree is less than MINFREE. MINFREE is defined in ufs/ffs/fs.h:
> 
> #define MINFREE  8
> 
> But relevant code in ufs/ffs/ffs_alloc.c uses hardcoded value:
> 
> 288 if (fs->fs_minfree <= 5 ||
> 289 fs->fs_cstotal.cs_nffree >
> 290 (off_t)fs->fs_dsize * fs->fs_minfree / (2 * 100))
> 291 break;
> 292 log(LOG_NOTICE, "%s: optimization changed from SPACE to TIME\n",
> 
> tunefs(8) metions 5% too.

The code you quote in ffs_alloc.c doesn't do what you think it
does.  The minfree comparison there is an exception to the rule
``if there is too much fragmentation, switch optimization from
space to time''.  The kernel must do this because the calculation
on lines 289 and 290 doesn't work very well if minfree is small.

The code in newfs works in the opposite direction: it forces
optimization from time to space if minfree is too small (<8%).
The rationale here is different: if you set minfree very low, you
must expect space to be very tight, so the filesystem should
optimize from space from the start.

So these numbers are different on purpose, because they address
separate issues.  The tunefs(8) comment you refer to seems to have
been introduced in rev. 1.2 of tunefs.8.  I revised the comment in
rev. 1.18, but even then I didn't catch the error.  I think we
need to s/of 5% and less/below the default value/.  The manpage
should also point out that the time/space optimization
restrictions based on minfree are enforced by newfs/tunefs when
minfree is changed, and not by the kernel.  They can be overridden
by subsequently asking for space or time optimization again.

Also, newfs does not match tunefs exactly in its treatment of
minfree.  Tunefs will additionally force time optimization if
minfree is >= the default.  Early comments in CVS logs seem to
indicate that tunefs's behavior was the intended one.

**OR**

Here is another idea: rip out of tunefs and newfs the magic that
forces space/time optimization.  Instead, when someone tries to
specify a small value for minfree, just warn them why they
shouldn't do that.  The one bit of minfree magic in ffs_alloc.c
stays because it's important.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: [PATCH]: newfs(8) FS_OPTSPACE vs FS_OPTTIME bug

2003-01-27 Thread David O'Brien
On Thu, Jan 23, 2003 at 01:42:58PM +0300, Maxim Konovalov wrote:
> Any objections to a diff below?

We should be moving away from magic numbers to #defined constants, not
the otherway around.
 
> Index: newfs/newfs.c
> ===
> RCS file: /home/ncvs/src/sbin/newfs/newfs.c,v
> retrieving revision 1.66
> diff -u -r1.66 newfs.c
> --- newfs/newfs.c 30 Nov 2002 18:28:26 -  1.66
> +++ newfs/newfs.c 23 Jan 2003 10:26:45 -
> - if (minfree < MINFREE && opt != FS_OPTSPACE) {
> + if (minfree <= 5 && opt != FS_OPTSPACE) {
> - fprintf(stderr, "because minfree is less than %d%%\n", MINFREE);
> + fprintf(stderr, "because minfree is less than %d%%\n", 5);

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



[PATCH]: newfs(8) FS_OPTSPACE vs FS_OPTTIME bug

2003-01-23 Thread Maxim Konovalov

Hello,

newfs(8) incorrectly claims that FS_OPTTIME is unavailable when
minfree is less than MINFREE. MINFREE is defined in ufs/ffs/fs.h:

#define MINFREE  8

But relevant code in ufs/ffs/ffs_alloc.c uses hardcoded value:

288 if (fs->fs_minfree <= 5 ||
289 fs->fs_cstotal.cs_nffree >
290 (off_t)fs->fs_dsize * fs->fs_minfree / (2 * 100))
291 break;
292 log(LOG_NOTICE, "%s: optimization changed from SPACE to TIME\n",

tunefs(8) metions 5% too.

Any objections to a diff below?

Index: newfs/newfs.c
===
RCS file: /home/ncvs/src/sbin/newfs/newfs.c,v
retrieving revision 1.66
diff -u -r1.66 newfs.c
--- newfs/newfs.c   30 Nov 2002 18:28:26 -  1.66
+++ newfs/newfs.c   23 Jan 2003 10:26:45 -
@@ -327,9 +327,9 @@
maxcontig = MAX(1, MAXPHYS / bsize);
if (density == 0)
density = NFPI * fsize;
-   if (minfree < MINFREE && opt != FS_OPTSPACE) {
+   if (minfree <= 5 && opt != FS_OPTSPACE) {
fprintf(stderr, "Warning: changing optimization to space ");
-   fprintf(stderr, "because minfree is less than %d%%\n", MINFREE);
+   fprintf(stderr, "because minfree is less than %d%%\n", 5);
opt = FS_OPTSPACE;
}
if (maxbpg == 0)

%%%

-- 
Maxim Konovalov, [EMAIL PROTECTED], [EMAIL PROTECTED]

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message