Re: kern/56332: swap on 4k sector device uses only 1/8 of the configured capacity

[mlelstv%serpens.de@localhost (Michael van Elst)]

The following reply was made to PR kern/56332; it has been noted by GNATS.

From: mlelstv%serpens.de@localhost (Michael van Elst)
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/56332: swap on 4k sector device uses only 1/8 of the configured capacity
Date: Wed, 28 Jul 2021 05:31:57 -0000 (UTC)

 tnn%nygren.pp.se@localhost writes:
 
 >ld4 at nvme0 nsid 1
 >ld4: 238 GB, 7752 cyl, 128 head, 63 sec, 4096 bytes/sect x 62514774 sectors
 >dk3 at ld4: 8388608 blocks at 50397440, type: swap
 >... but only 4 GiB is available.
 
 That looks like a bug in the dk driver.
 
 swap/dump are somewhat magic. Drivers have their own entry points to
 handle a swap (and dump) partition:
 
 DEVsize -> return the number of blocks
 DEVdump -> dump that many bytes to a given block number.
 
 Users of the driver will use DEV_BSIZE blocks, like for regular I/O.
 
 But dksize() returns sc_size and dkdump() checks block numbers against
 sc_size and offsets them by sc_offset. Both use the physical sector
 sizes (sc_size is 8388608, sc_offset is 50397440).
 
 So it's not only reporting the wrong size, but also writes to the
 wrong position on the disk if the physical sector size is not DEV_BSIZE.
 
 The regular I/O code does the right translation. So maybe (untested):
 
 Index: dk.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/dkwedge/dk.c,v
 retrieving revision 1.105
 diff -p -u -r1.105 dk.c
 --- dk.c	2 Jun 2021 17:56:40 -0000	1.105
 +++ dk.c	28 Jul 2021 05:31:14 -0000
 @@ -1639,6 +1639,7 @@ static int
  dksize(dev_t dev)
  {
  	struct dkwedge_softc *sc = dkwedge_lookup(dev);
 +	uint64_t p_size;
  	int rv = -1;
  
  	if (sc == NULL)
 @@ -1651,12 +1652,13 @@ dksize(dev_t dev)
  
  	/* Our content type is static, no need to open the device. */
  
 +	p_size   = sc->sc_size << sc->sc_parent->dk_blkshift;
  	if (strcmp(sc->sc_ptype, DKW_PTYPE_SWAP) == 0) {
  		/* Saturate if we are larger than INT_MAX. */
 -		if (sc->sc_size > INT_MAX)
 +		if (p_size > INT_MAX)
  			rv = INT_MAX;
  		else
 -			rv = (int) sc->sc_size;
 +			rv = (int) p_size;
  	}
  
  	mutex_exit(&sc->sc_parent->dk_rawlock);
 @@ -1675,6 +1677,7 @@ dkdump(dev_t dev, daddr_t blkno, void *v
  {
  	struct dkwedge_softc *sc = dkwedge_lookup(dev);
  	const struct bdevsw *bdev;
 +	uint64_t p_size, p_offset;
  	int rv = 0;
  
  	if (sc == NULL)
 @@ -1697,16 +1700,20 @@ dkdump(dev_t dev, daddr_t blkno, void *v
  		rv = EINVAL;
  		goto out;
  	}
 -	if (blkno < 0 || blkno + size / DEV_BSIZE > sc->sc_size) {
 +
 +	p_offset = sc->sc_offset << sc->sc_parent->dk_blkshift;
 +	p_size   = sc->sc_size << sc->sc_parent->dk_blkshift;
 +
 +	if (blkno < 0 || blkno + size / DEV_BSIZE > p_size) {
  		printf("%s: blkno (%" PRIu64 ") + size / DEV_BSIZE (%zu) > "
 -		    "sc->sc_size (%" PRIu64 ")\n", __func__, blkno,
 -		    size / DEV_BSIZE, sc->sc_size);
 +		    "p_size (%" PRIu64 ")\n", __func__, blkno,
 +		    size / DEV_BSIZE, p_size);
  		rv = EINVAL;
  		goto out;
  	}
  
  	bdev = bdevsw_lookup(sc->sc_pdev);
 -	rv = (*bdev->d_dump)(sc->sc_pdev, blkno + sc->sc_offset, va, size);
 +	rv = (*bdev->d_dump)(sc->sc_pdev, blkno + p_offset, va, size);
  
  out:
  	mutex_exit(&sc->sc_parent->dk_rawlock);