[PATCH] mkswap: handle 2^32 pages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Karel,

Your util-linux-ng v2.14.2-rc2 announcement reminds me that I've not
yet sent you the mkswap fix I promised when I sent in a kernel swapon
fix (73fd8748ab0b9b3ddd178bea1d7ae03372033d96 in Linus's 2.6.29 git):

swapfile: swapon needs larger size type
sys_swapon()'s swapfilesize (better renamed swapfilepages) is declared as
an int, but should be an unsigned long like the maxpages it's compared
against: on 64-bit (with 4kB pages) a swapfile of 2^44 bytes was rejected
with "Swap area shorter than signature indicates".

When I sent that to Andrew in November, I did say "mkswap needs its own
fixes for this: I'll be sending to Karel": sorry, only now do I do so.

Before getting down to the fix in question, let me mention in passing
two reasons I hadn't sent it on, two things I'd meant to investigate
further before contacting you, but haven't yet found the time for:

1. It looked to me as if mkswap and kernel (sys_swapon) disagree by 1
on what goes in the last_page field of the swap header - mkswap thinks
it's what I'd indeed call "last page", whereas kernel thinks it's a
total number of pages, so never actually uses that page.  The error
is the safe way round, it's not a big deal, and there are clustering
reasons why swap allocation would often tend not to use it anyway:
I probably don't dare change the kernel for it now.   But mention
it to you in case it's something you've noticed or can confirm; or
in case it comes from some relatively recent change in mkswap.c,
which we could then safely reverse - do you see any change to what
goes into last_page in recent mkswap history?  Or am I just confused
and off-by-one myself?

2. I've been disturbed to find the openSUSE 11.1 PowerPC swapon doing
mkswap in some circumstances (when I specify device rather than "-a"?
I'm not sure, I've not pinned it down yet).  I can understand why
they want that (PowerPC now supporting 4k and 64k and perhaps larger
page sizes: the current swap_header is badly designed for that), but
I do not like swapon modifying the swap header in such a way.  Okay,
I'm peculiar in setting up my swap size smaller then its partition,
for particular testing reasons; but I do think swapon should leave
it that way.  A quick look at your -rc2 source suggests that change
is nothing to do with you, I guess it comes from a SUSE patch but
I've not looked at their src.rpm yet.  Though your swapon may mkswap
for suspend reasons: hmm, wouldn't it do very much better just to
rewrite the swap signature, than exec mkswap - maybe nobody ever
uses the badpages list, but it really should be respected.

Anyway, those are the diversions I wanted to raise with you, but
on to the mkswap patch in question.  My kernel sys_swapon fix
originally came from source inspection: when I went to test it,
I had difficulty finding a filesystem which would allow a file
that big - surprised to find ext4 did not, but xfs did allow it.
Had to hack around mkswap's and kernel's tests for holes, since
I certainly don't have disks that big.  Managed in the end, but
had to fix mkswap too: its unsigned long pagecounts are not quite
big enough (when userspace is 32-bit) to handle the swap maxima.
Easiest fix was to change several to unsigned long long (or that's
what I'd do in the kernel: please keep in mind that it's years
since I've done much userspace programming, and I've little
grasp of its portability issues).

If you consider support for such enormous swap areas something
of a low priority, and prefer v2.14.2 without this, I'll find
it very hard to argue against you!

Hugh

[PATCH] mkswap: handle 2^32 pages

mkswap (when built 32-bit) could not quite support the maximum size
of swap area (2^32 pages): change unsigned long pagecounts to unsigned
long long pagecounts, and fix maxpages for the case when more is asked.

Signed-off-by: Hugh Dickins <hugh@xxxxxxxxxxx>

--- util-linux-ng-2.14.1/disk-utils/mkswap.c.0	2008-09-10 10:02:42.000000000 +0100
+++ util-linux-ng-2.14.1/disk-utils/mkswap.c	2008-11-15 22:17:40.000000000 +0000
@@ -36,6 +36,7 @@
 #include <string.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <limits.h>
 #include <mntent.h>
 #include <sys/ioctl.h>		/* for _IO */
 #include <sys/utsname.h>
@@ -59,7 +60,7 @@
 static char * program_name = "mkswap";
 static char * device_name = NULL;
 static int DEV = -1;
-static unsigned long PAGES = 0;
+static unsigned long long PAGES = 0;
 static unsigned long badpages = 0;
 static int check = 0;
 static int version = -1;
@@ -266,6 +267,7 @@ It is roughly 2GB on i386, PPC, m68k, AR
 */
 
 #define MAX_BADPAGES	((pagesize-1024-128*sizeof(int)-10)/sizeof(int))
+#define MIN_GOODPAGES	10
 
 /*
  * One more point of lossage - Linux swapspace really is a mess.
@@ -399,7 +401,7 @@ valid_offset (int fd, off_t offset) {
 	return 1;
 }
 
-static off_t
+static unsigned long long
 find_size (int fd) {
 	off_t high, low;
 
@@ -417,8 +419,8 @@ find_size (int fd) {
 	return (low + 1);
 }
 
-/* return size in pages, to avoid integer overflow */
-static unsigned long
+/* return size in pages */
+static unsigned long long
 get_size(const char  *file) {
 	int	fd;
 	unsigned long long size;
@@ -487,9 +489,9 @@ int
 main(int argc, char ** argv) {
 	struct stat statbuf;
 	int i;
-	unsigned long maxpages;
-	unsigned long goodpages;
-	unsigned long sz;
+	unsigned long long maxpages;
+	unsigned long long goodpages;
+	unsigned long long sz;
 	off_t offset;
 	int force = 0;
 	char *block_count = 0;
@@ -585,7 +587,7 @@ main(int argc, char ** argv) {
 		   explicitly */
 		char *tmp;
 		int blocks_per_page = pagesize/1024;
-		PAGES = strtoul(block_count,&tmp,0)/blocks_per_page;
+		PAGES = strtoull(block_count,&tmp,0)/blocks_per_page;
 		if (*tmp)
 			usage();
 	}
@@ -595,7 +597,7 @@ main(int argc, char ** argv) {
 	} else if (PAGES > sz && !force) {
 		fprintf(stderr,
 			_("%s: error: "
-			  "size %lu KiB is larger than device size %lu KiB\n"),
+			  "size %llu KiB is larger than device size %llu KiB\n"),
 			program_name,
 			PAGES*(pagesize/1024), sz*(pagesize/1024));
 		exit(1);
@@ -620,17 +622,17 @@ main(int argc, char ** argv) {
 		usage();
 	}
 
-	if (PAGES < 10) {
+	if (PAGES < MIN_GOODPAGES) {
 		fprintf(stderr,
 			_("%s: error: swap area needs to be at least %ld KiB\n"),
-			program_name, (long)(10 * pagesize/1024));
+			program_name, (long)(MIN_GOODPAGES * pagesize/1024));
 		usage();
 	}
 
 	if (version == 0)
 		maxpages = V0_MAX_PAGES;
 	else if (get_linux_version() >= KERNEL_VERSION(2,3,4))
-		maxpages = PAGES;
+		maxpages = UINT_MAX + 1ULL;
 	else if (get_linux_version() >= KERNEL_VERSION(2,2,1))
 		maxpages = V1_MAX_PAGES;
 	else
@@ -639,7 +641,7 @@ main(int argc, char ** argv) {
 	if (PAGES > maxpages) {
 		PAGES = maxpages;
 		fprintf(stderr,
-			_("%s: warning: truncating swap area to %ld KiB\n"),
+			_("%s: warning: truncating swap area to %llu KiB\n"),
 			program_name, PAGES * pagesize / 1024);
 	}
 
@@ -716,11 +718,11 @@ use the -f option to force it.\n"),
 		p->nr_badpages = badpages;
 	}
 
-	goodpages = PAGES - badpages - 1;
-	if ((long) goodpages <= 0)
+	if (badpages > PAGES - MIN_GOODPAGES)
 		die(_("Unable to set up swap-space: unreadable"));
+	goodpages = PAGES - badpages - 1;
 	printf(_("Setting up swapspace version %d, size = %llu KiB\n"),
-		version, (unsigned long long)goodpages * pagesize / 1024);
+		version, goodpages * pagesize / 1024);
 	write_signature((version == 0) ? "SWAP-SPACE" : "SWAPSPACE2");
 
 	if (version == 1)
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux