On Tue, 12 Feb 2008, Poul-Henning Kamp wrote:

> In message <47B181F2.2070808@icyb.net.ua>, Andriy Gapon writes:
>> 2.3. this code passes to bread blkno that is calculated as 4*sector,
>> where sector is a number of a physical 2048-byte sector. [**]
>> [**] - I think that this is a requirement of buffcache system, because
>> internally it performs many calculations that seem to assume that block
>> size is always 512.

> Yes, the buf-cache works in 512 bytes units throughout.

I missed the dbtob() conversions in vfs_bio.c when I replied previously
So blkno cannot be a cookie; it must be for a 512-block. So how did
the cases where bsize != DEV_BSIZE in getblk() ever work?

>> 3.1. for a fresh buf getlbk would assign the following:
>> bsize = bo->bo_bsize;
>> offset = blkno * bsize;

> That's clearly wrong.

If units were always 512-blocks, then anything except bsize = DEV_BSIZE
would be clearly wrong. Things aren't that simple (but probably should
be). Even RELENG_3 has bsize = f_iosize (possibly != 512) for non-disks.
That seems to include nfs(client). In fact, nfs_getcacheblk() does
weird scaling which seems to be mainly to compensate for for weird scaling
here. It calls getblk() with a bn arg that seems to be f_iosize units.
Then at then end, for the VREG case, it sets bp->b_blkno to this bn
scaled to normal DEV_BSIZE units. bp->b_blkno seems to have DEV_BSIZE
units for all uses of it in nfs.

> We need to assert that the blkno is aligned to the start of a sector
> and use the 512 byte units, so I guess it would be:
> offset = dbtob(blkno);
> KASSERT(!(offset & (bsize - 1)), ("suitable diagnostic"));

Barely worth checking.

The current bug has nothing to do with this. The offset is just wrong
at this point, after using a scale factor that is inconsistent with the
units of blkno, for all (?) disk (and other?) file systems whose sector
size isn't 512.

freebsd-hackers@freebsd.org mailing list
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"