Re: vnode_has_large_blocks() (vnd.c rev 1.255)

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Wed, Sep 05, 2018 at 08:28:27PM -0000, Michael van Elst wrote:
> bouyer%antioche.eu.org@localhost (Manuel Bouyer) writes:
> 
> >Hello,
> >in vnd.c rev 1.255, vnode_has_large_blocks() has been introduced, and
> >will cause vnd to use VOP_READ/VOP_WRITE instead of VOP_BMAP/VOP_STRATEGY
> >if it returns true. The commit message was:
> >Fall back to VOP_READ/VOP_WRITE if the simulated disk has smaller
> >sectors than the underlying filesystem and VOP_STRATEGY would fail.
> 
> >But actually, vnode_has_large_blocks() always return true because at this
> >point, getdisksize() always fail: the vnd is not yet initialized so we
> >can't read its wegdes or disklabel information.
> 
> vnode_has_large_blocks compares the backing store (that's what
> getdisksize queries) against the geometry of the vnd unit.
> 
> The backing store and the geometry are initialized before vndthread
> is started, getdisksize() shouldn't fail and I'm sure it didn't
> at that time.

AFAIK getdisksize() returns the parameters of the vnd device, not the
backing store. For this it needs to read the vnd disklabel, and
for read to work, it needs the vnd thread. The ioctl fails because
VNF_INITED is not set. getdisksize() can't work before the thread is
fully up and running.


> 
> >What is, exactly, the purpose of this commit and what problem does it tries
> >to solve ? AFAIK, handle_with_strategy() will split the request as
> >appropriate if it's too large for the underlying filesystem.
> 
> vnd has two ways to access the backing store.
> 
> It either uses vnrdwr() in handle_with_rdwr, all I/O size or alignment
> issues are handled by the filesystem.
> 
> Or it uses handle_with_strategy where an I/O request is sent by
> mapping the block offset and sending appropriate I/O requests
> to the backing store using the strategy routine.
> 
> However, the strategy routine can only handle blocks of specific
> size (bsize = vnd->sc_vp->v_mount->mnt_stat.f_iosize).
> 
> If the I/O request is larger, vnd splits it into multiple requests.
> 
> But if the I/O request is smaller, it would fail. The vnode_has_large_blocks
> validates that this may happen and configures vnd to fall back to the
> slower method. This happens when the backing store is e.g. on a 4k/sector
> disk and the vnd device simulates a 512byte/sector disk.

OK, I though this would be the filesystem block or fragment size.
This makes sense because in my case, the vnd and physical disk both
have 512 bytes sectors.

But in any case, the code as is is wrong, and always ends up using 
vnrdwr(). The test has to be done after the thread is fully set up.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--