tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: [PATCH] Re: zero-filed page on VOP_PUTPAGES


> YAMAMOTO Takashi <> wrote:
>> > It just prevents puffs_vnop_getattr to call uvm_vnp_setattr with a stall
>> > value between the time dosetattr sends PUFFS_VN_SETATTR and the time it
>> > calls uvm_vnp_setattr. This is enough to work around the issue if the
>> > file server does not reorder requests. 
>> but file servers can reorder requests, can't they?
> It is not specified. I observed reordering in the past, but I now
> suspect that is was caused at PUFFS level, because puffs_vn_getattr()
> was working on an unlocked vnode, and therefore could slice itself
> within dosetattr() operations, and resize the file with the value it got
> from the filesystem. 
> Anyway, if the fileserver reorder file size set and write operations on
> its own, I see no way it can avoid data corruption by spurious truncate.
> I guess this should be called a bug in the file server.

it makes sense.
i tend to say protocol bug, tho.

>> besides that it's strongly discouraged way (i'd call it bug) to use locks,
> I came up with that solution when working with the pn->pn_inresize thing
> (used to keep track of the threads involved in a pending
> PUFFS_VN_SETATTR, for whoever is joining the discussion). I noticed that
> I needed to prevent race on accesses to pn->pn_inresize. This involved
> locking the vnode in puffs_vnop_getattr(), and I realized that the lock
> itself was enough to prevent the race on file resize operations.
> To sum it up: I think that puffs_vnop_getattr() must lock the vnode
> since it sends a request to the filesystem and then call
> uvm_vnp_setsize() with the size it got. Let the vnode unlocked in that
> windows is just asking for race conditions.
>> the test seems broken when called with LK_SHARED held.
> What happens if vn_lock(vp, LK_EXCLUSIVE|LK_RETRY) is called on a
> LK_SHARED locked vnode? I would expect it to sleep until the shared lock
> is released.

a recursive locking attempt is an error.

if we go this route, it's cleaner to use a puffs internal lock, or change
the locking protocol to require at least LK_SHARED for VOP_GETATTR.
cf. vfs_syscalls.c rev.1.350.2.3


> -- 
> Emmanuel Dreyfus

Home | Main Index | Thread Index | Old Index