I have updated the comments, new webrev is here (it also contains the latest umountall changeset from today):
http://cr.opensolaris.org/~pavelf/6778894-v3 Can you give an explicit review of this workspace? Thanks, Pavel Frank Batschulat (Home) wrote: > On Wed, 17 Dec 2008 15:13:40 +0100, Pavel Filipensky <Pavel.Filipensky at > sun.com> wrote: > > >> Two notes: >> >> 1) >> I am wondering if I should add this comment as a warning for future >> code changes >> which would go OTW and cause regression of "6675447 NFSv4 client hangs >> on shutdown if server is down beforehand." >> >> # we cannot us 'df -F nfs $mountp' to test if the given mountpoint has >> NFS on top of it >> # since df could trigger NFS over-the-wire request and cause regression of >> # "6675447 NFSv4 client hangs on shutdown if server is down beforehand." >> > > yeah I really do think we should put a big fat warning inthere, > that the use of any syscall on a NFS filesystem has the danger to go OTW > and thus defeat the whole purpose of what we're trying to achive right now. > > >> 2) >> The more I am looking to umountall(1M) the more I think that umountall(1M) >> should get a new syscall and all the work apart from parsing the command >> line options should be done in the kernel. >> >> Now, the kernel exports its state via a special file system in /etc/mnttab. >> Information from /etc/mnttab is parsed as a text in umountall(1M) - parsing >> is error prone - space can do lot of harm - see 6786256. After the >> information is parsed >> it is processed with low efficiency just to get a list of mount points >> to be unmounted. >> This list goes back to the kernel via umount(1). Current shell >> implementation is slow and hard to maintain. >> > > this has been annoying me for a long time, we do have the in kernel mnttab > and we do have the inkernel sharetab ! whats the story about this ancient > shell script > anyways these days ! > > I think we definitively should at least file a place holder RFE that we > have a record of this would be a good idea to re-architect. > > --- > frankB >
