aligned_nblks calculations broken in vm_swap.c

2003-01-03 Thread Bruce Evans
% Index: vm_swap.c
% ===
% RCS file: /home/ncvs/src/sys/vm/vm_swap.c,v
% retrieving revision 1.127
% retrieving revision 1.128
% diff -u -1 -r1.127 -r1.128
% --- vm_swap.c 3 Jan 2003 09:55:05 -   1.127
% +++ vm_swap.c 3 Jan 2003 14:30:46 -   1.128
% ...
% @@ -353,3 +352,3 @@
%*/
% - aligned_nblks = (nblks + (dmmax - 1))  ~(u_long)(dmmax - 1);
% + aligned_nblks = (nblks + dmmax_mask)  ~(u_long)dmmax_mask;
%
% @@ -472,3 +471,3 @@
%
% - aligned_nblks = (nblks + (dmmax - 1))  ~(u_long)(dmmax - 1);
% + aligned_nblks = (nblks + dmmax_mask)  ~(u_long)dmmax_mask;
%   nswap = aligned_nblks * nswdev;

dmmax_mask is ~(dmmax - 1) not (dmmax - 1), so all of these changes are
wrong.

(New) nearby style bugs: removing the use of dmmax bogotifies a comment
which refers to dmmax.

(Old) related type bugs: dmmax_mask has the bogus type `int', so it is not
clear how it works as a mask on machines with 32-bit ints and 64-bit
block numbers.  I think it works because all supported machines are 2's
complement; this makes the negative value obtained by complementing the
bits of (dmmax - 1) right for masking with ints, and C's integral promotion
rules then make it right for masking with larger integral types too.
~(u_long)(dmmax - 1) in the old code was more careful about this, except it
only works for u_long and smaller block numbers (which is enough for
aligned_nblks).

Bruce


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



Re: aligned_nblks calculations broken in vm_swap.c

2003-01-03 Thread phk
In message [EMAIL PROTECTED], Bruce Evans writes:
% Index: vm_swap.c
% ===
% RCS file: /home/ncvs/src/sys/vm/vm_swap.c,v
% retrieving revision 1.127
% retrieving revision 1.128
% diff -u -1 -r1.127 -r1.128
% --- vm_swap.c3 Jan 2003 09:55:05 -   1.127
% +++ vm_swap.c3 Jan 2003 14:30:46 -   1.128
% ...
% @@ -353,3 +352,3 @@
%   */
% -aligned_nblks = (nblks + (dmmax - 1))  ~(u_long)(dmmax - 1);
% +aligned_nblks = (nblks + dmmax_mask)  ~(u_long)dmmax_mask;
%
% @@ -472,3 +471,3 @@
%
% -aligned_nblks = (nblks + (dmmax - 1))  ~(u_long)(dmmax - 1);
% +aligned_nblks = (nblks + dmmax_mask)  ~(u_long)dmmax_mask;
%  nswap = aligned_nblks * nswdev;

dmmax_mask is ~(dmmax - 1) not (dmmax - 1), so all of these changes are
wrong.

damn!

Fixed, thanks!

(Old) related type bugs:

I'm actually more than a bit of mind to rip out the entire bogus
swap-stripe code:  If you want swap on a striped disk, you should
use hardware, controller, vinum, ccd or raidframe to stripe.

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

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



Re: aligned_nblks calculations broken in vm_swap.c

2003-01-03 Thread David Schultz
Thus spake [EMAIL PROTECTED] [EMAIL PROTECTED]:
 I'm actually more than a bit of mind to rip out the entire bogus
 swap-stripe code:  If you want swap on a striped disk, you should
 use hardware, controller, vinum, ccd or raidframe to stripe.

Ccd is a nice simple solution, but by using it you lose the
ability to dynamically add and remove swap and play other tricks
like swap device prioritization.  (Actually, prioritization would
require a better abstraction for the swap subsystem anyway.)  On
the other hand, the static limit on the number of devices is
annoying.  But the point is that the swap striping code isn't
entirely redundant.

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