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>
index | next in thread | previous in thread | raw e-mail
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
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080212234018.O92415>