Frank Hofmann wrote: > Sorry, wrong mail domain :) > FrankH. > > > ---------- Forwarded message ---------- > Date: Fri, 28 Sep 2007 14:21:54 +0100 (BST) > From: Frank Hofmann <Frank.Hofmann at Sun.COM> > To: Moinak Ghosh <Moinak.Ghosh at Sun.COM> > Cc: hsfs-iteam at Sun.COM, caiman-discuss at Sun.COM > Subject: Re: Final webrevs updated for HSFS readahead changes > > > Hi Moinak, > > to me, the code looks great now. Except for one thing: > > As far as the kstats go, what's the system doing on multiple kstats being > created with the same name ? You're doing the kstat_create once per mount, > but > re-use the same name/ID fields. I think that might cause memory leaks; has > the > hsfs kstat code been tested with 2+ concurrently mounted/accessed HSFS > filesystems ? > > If you take e.g. UFS as a precedence (inode / logging / directio kstats), all > of these are created at module initialization, not at mount. Same for the ZFS > arc kstats. > There's other precedence (fssnap) for per-mountpoint kstats, but if you do > that > you'd have to: > - make sure you pass unique names / IDs > - delete previously-existing kstats of the same name / ID > when you call kstat_create(). It's more complicated and there's the > possibility > for errors. > > The kstats are nice to have, right there; but if it gets too complicated to > make them per-mountpoint then either make them global, or simply purge the > kstats, get a few more SDT probes in and let DTrace do the sum() instead of > the > kstat framework. It's probably not worth spending another week on this, it's > a > side aspect of the change. >
Thanks for the comments. I think I can easily change the kstats to be global and also run tests with multiple mounts. The kstats are useful as they capture information during normal operation and provide snapshots of behavior till date, whereas DTrace captures information only when tracing. Regards, Moinak. > Thanks for the great work on this ! > FrankH. > > > On Thu, 20 Sep 2007, Moinak Ghosh wrote: > > >> I have completed all the updates after the last review comments and completed >> all my testing. I have done manual tests and Filebench tests on SPARC and >> x86. The Filebench tests were also repeated to kmem_flags set to 0x1f and >> checked for correctness and leaks. The fsfuzzer test was run for 10days >> without >> issues. The bug is updated. >> >> The updated webrevs are available at: >> http://www.genunix.org/distributions/belenix_site//binfiles/hsfswebrev/index.htm >> >> Updated binaries available from: >> http://www.genunix.org/distributions/belenix_site//binfiles/hsfs.amd64 >> http://www.genunix.org/distributions/belenix_site//binfiles/hsfs.sparcv9 >> >> This update adds the following changes: >> >> - Incorporate all the review comments. >> - Add kstats and SDT probes for debugging purposes. You can copy the >> updated module, mount a DVD and see: >> kstat -m hsfs_fs >> dtrace -l -P sdt | grep hsfs >> - SPARC testing was done on a T2000 and it threw up a surprise perf >> issue for the single-stream sequential read and random read tests. It was >> found using standard kmem_alloc for thousands of small allocations of >> size 96 bytes was inefficient. So a kmem_cache was used and it made >> a dramatic difference. >> - Removed one unnecessary page_lookup in the readahead path. >> >> As it stands now there is an across the board improvement on all Filebench >> tests including the random read tests on both x86 and SPARC. >> >> Please send your replies to caiman-discuss where this email was posted as >> well. >> >> Regards, >> Moinak. >> >> > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >