Source-Changes-D archive

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

Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs



On Fri, Oct 30, 2009 at 08:32:30AM +0100, Adam Hamsik wrote:
> 
> On Oct,Friday 30 2009, at 12:25 AM, Bill Stouder-Studenmund wrote:
> 
> >On Thu, Oct 29, 2009 at 11:58:32PM +0100, Adam Hamsik wrote:
> >>
> >>On Oct,Thursday 29 2009, at 8:15 PM, David Laight wrote:
> >>
> >>>On Wed, Oct 28, 2009 at 11:44:51PM +0000, Adam Hamsik wrote:
> >>>>Module Name:      src
> >>>>Committed By:     haad
> >>>>Date:             Wed Oct 28 23:44:51 UTC 2009
> >>>>
> >>>>Modified Files:
> >>>>  src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c
> >>>>zfs_znode.c
> >>>>
> >>>>Log Message:
> >>>>Add workaround about zfs vnode reclaiming deadlock by checking if
> >>>>we don't
> >>>>ehld ZFS_MUTEX_OBJ already. If we can lock OBJ_MUTEX deffer
> >>>>execution of
> >>>>zfs_zinactive to taskq. Code was inspired by FreeBSD
> >>>>zfs_freebsd_reclaim.
> >>>
> >>>This doesn't sound right at all ...
> >>>
> >>>If the 'If we can lock OBJ_MUTEX' test is a try_mutex_enter() then
> >>>it may fail because another lwp owns it ...
> >>>I hope it is only the checkin comment that is inverted!
> >>
> >>There are 3 option which can happen
> >>
> >>1) Our lwp already holds mutex
> >>2) mutex is not held at all
> >>3) Another LWP holds it
> >>
> >>In case 1 we can call reclaim vnode, in case 2 we lock it with
> >>try_mutex_enter and reclaim it and in case 3 we use taskq for
> >>reclaiming of vnode.
> >
> >This is reclaiming in response to someone trying to get a cleaned  
> >vnode,
> >correct? If so, we should just change the reclaim API and return  
> >something
> >like EBUSY. If it's just cleaning to clean, then having a taskq do  
> >it is
> >fine.
> 
> That can be done but ti will require to change all fses +  
> getcleanvnode because currently when it can't clean vnode it panics.

Yes, you have to change getcleanvnode, but why do the file systems need to 
change? That's making this harder than it has to be.

Pick an error value. I now thing EDEADLK is best, since it describes 
what's going on. Make getcleanvnode just skip to the next one in the list 
if it gets this error value back. Let it panic otherwise.

The only thing you need to do with other file systems is make sure their 
reclaim doesn't report EDEADLK when the code really wants to panic the 
kernel. Since we don't hear a lot about panicing while getting new vnodes 
(other than this bug), I think you're safe.

Take care,

Bill

Attachment: pgpKtJdB89E2B.pgp
Description: PGP signature



Home | Main Index | Thread Index | Old Index