[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 <yamt%mwd.biglobe.ne.jp@localhost> 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
Main Index |
Thread Index |