On 03/17/10 04:10 PM, Thomas Whitten wrote: > Thanks again, Antonello. There are a few responses inline. > > I think that we're pretty much in agreement now, but please shout if you > disagree. Yes we are. Thanks for addressing my comments.
Antonello > > tom > > Antonello Cruz writes: >> On 03/17/10 11:46 AM, Thomas Whitten wrote: >> ... >>> Antonello Cruz writes: >> ... >>>> usr/src/cmd/svc/configd/backend.c >>>> flight_recorder_event() > ... >>>> >>>> The mutex synchronizes access to the counters only? What about >>>> the be_flight_event_t data itself? Doesn't it need to be >>>> synchronized? >>> >>> The locks protect the selection of which be_flight_event_t we are going to >>> write by setting item while under lock. The only potential problem is if >>> we completely wrap around to the selection before the update is completed. >>> This is unlikely, but possible if as you say below we have a large number >>> of threads. To fix this, I think that I would like to get rid of the wrap >>> around all together. It is the early boot events that are the most >>> interesting and hardest to capture without the flight recorder, so I don't >>> think that over writing them makes sense. Once the system is up and >>> running there are other ways to capture information. >> Getting rid of the wrap around will bring sense, at least in my >> perspective, to flight_recorder_missed. Is there any operation that >> would generate flight recorder events once emi and lmi are finished? >> wouldn't restarting manifest import generate more events? What about a >> sysadmin creating a repository backup? I am just trying to understand if >> it makes sense to keep logging these events or not. If not, you should >> probably make it a no op when you reach MAX_FLIGHT_RECORDER_EVENTS. > > More events can be generated after emi and lmi are finished. Your examples > are correct. If we have a problem in these areas, though, we can use > dtrace to capture information. The early boot events are hard to capture, > and that was my main motivation for creating the flight recorder. > >> >>> > ... >>> >>> Any suggestions on improving the comments in 92-96? >> I have two suggestions: >> In the block 92-96 you could say somewhere that when the BE (backend) >> points to a volatile storage, be_ppath points to its original persistent >> location. Otherwise it is NULL. >> >> Also, in line 103, you could add: when backend is volatile >> > > Good suggestions. I'll incorporate them. > >>> >>>> > ... >>>> >>>> On 03/ 9/10 06:30 PM, Tony Nguyen wrote: >>>>> An updated webrev is generated to reflect the comments we've had thus far: >>>>> >>>>> http://cr.opensolaris.org/~tonyn/emi_mar_08_2010/ >>>>> >>>>> The last webrev is still available at >>>>> >>>>> http://cr.opensolaris.org/~tonyn/EMI_webrev >>>>> >>>>> Thanks, >>>>> tn >>>>> >>>>>> Subject: Early Manifest Import - code review request >>>>>> Date: Fri, 05 Feb 2010 14:21:46 -0800 >>>>>> From: Tony Nguyen<Truong.Q.Nguyen at sun.com> >>>>>> To: Antonello Cruz<Antonello.Cruz at Sun.COM>, Liane Praza >>>>>> <Liane.Praza at Sun.COM>, Susan Kamm-Worrell >>>>>> <Susan.Kamm-Worrell at Sun.COM>, Jean McCormack<Jean.McCormack at >>>>>> Sun.COM>, >>>>>> smf-discuss<smf-discuss at opensolaris.org> >>>>>> >>>>>> >>>>>> >>>>>> We completed testing and fixed all found bugs. We'd appreciate feedback >>>>>> by Feb 19th as that would allow us to turnaround comments and still make >>>>>> the scheduled integration date. >>>>>> >>>>>> Webrev: >>>>>> http://cr.opensolaris.org/~tonyn/EMI_webrev/ >>>>>> >>>>>> Design Doc: >>>>>> http://hub.opensolaris.org/bin/download/Community+Group+smf/smf_design_docs/emidesign.html
