Date: Wed, 13 Feb 2008 00:11:28 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: freebsd-fs@freebsd.org, freebsd-hackers@freebsd.org, Pav Lucistnik <pav@freebsd.org>, Andriy Gapon <avg@icyb.net.ua>, Bruce Evans <bde@zeta.org.au> Subject: Re: fs/udf: vm pages "overlap" while reading large dir Message-ID: <20080212234018.O92415@delplex.bde.org> In-Reply-To: <4299.1202816854@critter.freebsd.dk> References: <4299.1202816854@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080212234018.O92415>