Please see my answer to Hans Feldt comment. Thanks /Lennart
> -----Original Message----- > From: Ramesh Betham [mailto:[email protected]] > Sent: den 8 juli 2013 06:31 > To: Lennart Lund > Cc: Hans Feldt; Anders Widell; [email protected] > Subject: Re: [devel] [PATCH 0 of 7] Review request: logsv: Fix hanging main > thread when file i/o don't return > > Hi Lennart, > > My 2 cents... > > In my opinion, this is one way of handling the > parameters/messaging/synchronizations etc. between threads. But not able > to correlate why it is not suitable to implement this with LEAP mailbox. > > At developers discretion, different kinds of implementation can be even > followed for other threads that exist today in OpenSAF services. But the > architecture, design, implementation of OpenSAF services took some > common approach. It is better to maintain consistency even for new feature > implementations, unless and until if it is not viable (or) has drastic > performance impacts. > > Best Regards, > Ramesh. > > On 7/5/2013 8:20 PM, Lennart Lund wrote: > > In principle I agree with Hans that the best way of implementing inter > thread communication is to use a common design pattern (in OpenSAF LEAP > mailbox). This is for example if there are two threads running in parallel > that > needs to communicate and the main synchronization is one thread waiting > for a message from the other thread, but this is not the case in here. > > > > In order to protect the log server "main thread" (MT) from hanging if a file > operation like write, mkdir etc. does not return, all such operations are done > in a separate "file thread" (FT). > > Functions running in the "Main Thread" (MT) that needs file system > operations handle over the execution to the FT when file handling has to be > done. > > Execution is then given back to the MT again. If a file operation does not > return FT will hang but MT will time out the FT and resume. A timeout will be > handled as a file operation fail. > > The MT can detect if the FT is hanging and new requests for file operations > will be "failed". > > > > The main "communication" between the MT and the FT is synchronization. > The MT is suspended, FT takes over execution (timed out if hanging), MT is > resumed etc. This is handled with condition variable, mutex and > synchronization flags. There is also some parameters that has to be sent from > MT to FT and from FT to MT (this is done very simple). This parameter > handling must also be part of the synchronization, e.g there must be no risk > of that a mutex is locked if the FT hangs. LEAP mailbox is not suitable for > this > handling. The code for doing this is not very complicated. Everything is done > within two functions (file_hndl_thread and log_file_api) in file lgs_file.c. > The > solution is discussed an confirmed by Anders Widell. > > > > /Lennart > > > > > >> -----Original Message----- > >> From: Hans Feldt > >> Sent: den 5 juli 2013 11:59 > >> To: Lennart Lund > >> Cc: [email protected]; opensaf- > [email protected] > >> Subject: RE: [devel] [PATCH 0 of 7] Review request: logsv: Fix > >> hanging main thread when file i/o don't return > >> > >> The LEAP mailbox is not "overkill", it removes the need for locks etc > >> in each thread. It is a design pattern used by all services including > >> LOG itself. Breaking such pattern I don't think is OK for zero > >> benefit. Likely the other way around, introducing new problems and more > code for locking etc. > >> /Hans > >> > >>> -----Original Message----- > >>> From: Lennart Lund > >>> Sent: den 5 juli 2013 09:47 > >>> To: Hans Feldt > >>> Cc: [email protected]; > >>> [email protected] > >>> Subject: RE: [devel] [PATCH 0 of 7] Review request: logsv: Fix > >>> hanging main thread when file i/o don't return > >>> > >>> In this case is the messages are sent in both directions. However it > >>> is very "simple" messages and no message queue is needed. To use > >>> LEAP > >> mailbox would be "overkill" > >>> /Lennart > >>> > >>>> -----Original Message----- > >>>> From: Hans Feldt > >>>> Sent: den 20 juni 2013 13:46 > >>>> To: Lennart Lund > >>>> Cc: [email protected]; > >>>> [email protected] > >>>> Subject: Re: [devel] [PATCH 0 of 7] Review request: logsv: Fix > >>>> hanging main thread when file i/o don't return > >>>> > >>>> Why isn't the same design pattern used as in all other services in > >>>> opensaf - a LEAP mailbox for *message passing* between threads? > >>>> > >>>> Thanks, > >>>> Hans > >>>> > >>>> On 06/18/2013 02:54 PM, Lennart Lund wrote: > >>>>> Summary: logsv: Fix hanging main thread when file i/o don't return > >>>>> Review request for Trac Ticket(s): #9 Peer Reviewer(s): Madhurika > >>>>> Koppula, (Anders Widell, Hans Feldt) Pull request to: NA Affected > >>>>> branch(es): devel (4.4) Development branch: <<IF ANY GIVE THE > REPO > >>>>> URL>> > >>>>> > >>>>> -------------------------------- > >>>>> Impacted area Impact y/n > >>>>> -------------------------------- > >>>>> Docs n > >>>>> Build system n > >>>>> RPM/packaging n > >>>>> Configuration files n > >>>>> Startup scripts n > >>>>> SAF services y > >>>>> OpenSAF services n > >>>>> Core libraries n > >>>>> Samples n > >>>>> Tests n > >>>>> Other n > >>>>> > >>>>> > >>>>> Comments (indicate scope for each "y" above): > >>>>> --------------------------------------------- > >>>>> All file operations are handled in a separate thread. Functions > >>>>> doing operations on file system are changed so that this > >>>>> operations are done by a "handler" that is running in a separate > >>>>> thread. If a file operation "hangs" it will be timed out and an > >>>>> error is reported back to the > >>>> main thread. > >>>>> NOTE: > >>>>> In order to simplify retesting and troubleshooting some test > >>>>> traces and test code (tagged with LLDTEST) is not yet removed. > >>>>> This will be > >>>> removed or changed to "correct" > >>>>> TRACEs and LOGs before pushing. > >>>>> > >>>>> changeset 2296ad137a4f783c3efc69adf8db7261697aa327 > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:35:03 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 1 > >>>>> > >>>>> Generic thread handling: > >>>>> - Generic thread handling > >>>>> - Test handlers and two "real" handlers implemented > >>>>> > >>>>> changeset a6c505373ce8925fe2b8bf100c864b27d7f14b72 > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:35:52 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 2 > >>>>> > >>>>> More functions converted to use threaded file handling. > >>>>> - get_number_of_log_files_hdl() > >>>>> - Cleaning up init handling > >>>>> > >>>>> changeset e7119938da9f307f20aab9894934be08a122941f > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:39:00 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 3 > >>>>> > >>>>> More functions converted to use threaded file handling. > >>>>> - write log record function > >>>>> > >>>>> changeset 0574270ab6bf3a65548a8d23ec2989e9f26d67aa > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:45:34 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 4 > >>>>> > >>>>> More functions converted to use threaded file handling. > >>>>> - create_config_file_hdl > >>>>> > >>>>> changeset b989e158e38afa8fc231f40d0a32262611a334d2 > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:45:49 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 5 > >>>>> > >>>>> More functions converted to use threaded file handling. > >>>>> - lgs_file_rename_hfop(..) > >>>>> > >>>>> changeset aa83514345401e13c648d7d78940fc3b50f05636 > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:45:50 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 6 > >>>>> > >>>>> More functions converted to use threaded file handling: > >>>>> - check_path_exists_hdl(..) > >>>>> - This is the first inc for error handling updates. > >>>>> > >>>>> changeset 54b294afdfdc79c3fda2ef925939fc82633ce6ab > >>>>> Author: Lennart Lund <[email protected]> > >>>>> Date: Tue, 18 Jun 2013 12:45:50 +0200 > >>>>> > >>>>> logsv: Fix hanging main thread when file i/o don't return. Part > >>>>> 7 > >>>>> > >>>>> - Handling of object implementer rejects > >>>>> - Invalidate stream fd if errno EBADF when writing log record > >>>>> - Fix Error handling for too long path (> PATH_MAX) > >>>>> - Rename function that got temporary names during devel > >>>>> - Functions that uses a handler in file thread has got extension > >>>>> _h > >>>>> > >>>>> > >>>>> Added Files: > >>>>> ------------ > >>>>> README_LOGENH > >>>>> osaf/services/saf/logsv/lgs/lgs_file.c > >>>>> osaf/services/saf/logsv/lgs/lgs_file.h > >>>>> osaf/services/saf/logsv/lgs/lgs_filehdl.c > >>>>> osaf/services/saf/logsv/lgs/lgs_filehdl.h > >>>>> > >>>>> > >>>>> Complete diffstat: > >>>>> ------------------ > >>>>> osaf/services/saf/logsv/lgs/Makefile.am | 8 +- > >>>>> osaf/services/saf/logsv/lgs/lgs.h | 3 +- > >>>>> osaf/services/saf/logsv/lgs/lgs_amf.c | 10 +- > >>>>> osaf/services/saf/logsv/lgs/lgs_cb.h | 2 + > >>>>> osaf/services/saf/logsv/lgs/lgs_evt.c | 30 +++--- > >>>>> osaf/services/saf/logsv/lgs/lgs_evt.h | 4 + > >>>>> osaf/services/saf/logsv/lgs/lgs_file.c | 421 > >> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> +++++++++++++++++++++++++++++++++++ > >>>>> osaf/services/saf/logsv/lgs/lgs_file.h | 121 > >>>> ++++++++++++++++++++++++++ > >>>>> osaf/services/saf/logsv/lgs/lgs_filehdl.c | 377 > >> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>> +++++++++++++++++++++++++ > >>>>> osaf/services/saf/logsv/lgs/lgs_filehdl.h | 30 ++++++ > >>>>> osaf/services/saf/logsv/lgs/lgs_imm.c | 87 +++++++++++------- > >>>>> osaf/services/saf/logsv/lgs/lgs_main.c | 14 ++- > >>>>> osaf/services/saf/logsv/lgs/lgs_mbcsv.c | 16 +- > >>>>> osaf/services/saf/logsv/lgs/lgs_stream.c | 338 > >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++------- > -- > >> ----- > >>>> ---------- > >>>>> osaf/services/saf/logsv/lgs/lgs_stream.h | 27 +++-- > >>>>> osaf/services/saf/logsv/lgs/lgs_util.c | 105 > >> ++++++++++++++++++++-- > >>>>> osaf/services/saf/logsv/lgs/lgs_util.h | 15 ++- > >>>>> 17 files changed, 1401 insertions(+), 207 deletions(-) > >>>>> > >>>>> > >>>>> Testing Commands: > >>>>> ----------------- > >>>>> logtest can be used to run a regression test. > >>>>> > >>>>> > >>>>> Testing, Expected Results: > >>>>> -------------------------- > >>>>> All tests shall pass. > >>>>> If the file handler thread is "hanging" then > >>>>> - tests that writes log records will fail > >>>>> - IMM configuration of log related objects that requires > >>>>> access to file system will fail > >>>>> > >>>>> > >>>>> Conditions of Submission: > >>>>> ------------------------- > >>>>> Ack by Madhurika Koppula > >>>>> > >>>>> > >>>>> Arch Built Started Linux distro > >>>>> ------------------------------------------- > >>>>> mips n n > >>>>> mips64 n n > >>>>> x86 n n > >>>>> x86_64 n n > >>>>> powerpc n n > >>>>> powerpc64 n n > >>>>> > >>>>> > >>>>> Reviewer Checklist: > >>>>> ------------------- > >>>>> [Submitters: make sure that your review doesn't trigger any > >>>>> checkmarks!] > >>>>> > >>>>> > >>>>> Your checkin has not passed review because (see checked entries): > >>>>> > >>>>> ___ Your RR template is generally incomplete; it has too many > >>>>> blank > >> entries > >>>>> that need proper data filled in. > >>>>> > >>>>> ___ You have failed to nominate the proper persons for review and > >> push. > >>>>> ___ Your patches do not have proper short+long header > >>>>> > >>>>> ___ You have grammar/spelling in your header that is unacceptable. > >>>>> > >>>>> ___ You have exceeded a sensible line length in your > >>>> headers/comments/text. > >>>>> ___ You have failed to put in a proper Trac Ticket # into your commits. > >>>>> > >>>>> ___ You have incorrectly put/left internal data in your comments/files > >>>>> (i.e. internal bug tracking tool IDs, product names etc) > >>>>> > >>>>> ___ You have not given any evidence of testing beyond basic build > >> tests. > >>>>> Demonstrate some level of runtime or other sanity testing. > >>>>> > >>>>> ___ You have ^M present in some of your files. These have to be > >> removed. > >>>>> ___ You have needlessly changed whitespace or added whitespace > >> crimes > >>>>> like trailing spaces, or spaces before tabs. > >>>>> > >>>>> ___ You have mixed real technical changes with whitespace and other > >>>>> cosmetic code cleanup changes. These have to be separate > commits. > >>>>> > >>>>> ___ You need to refactor your submission into logical chunks; there is > >>>>> too much content into a single commit. > >>>>> > >>>>> ___ You have extraneous garbage in your review (merge commits > etc) > >>>>> > >>>>> ___ You have giant attachments which should never have been sent; > >>>>> Instead you should place your content in a public tree to be > >>>>> pulled. > >>>>> > >>>>> ___ You have too many commits attached to an e-mail; resend as > >> threaded > >>>>> commits, or place in a public tree for a pull. > >>>>> > >>>>> ___ You have resent this content multiple times without a clear > >> indication > >>>>> of what has changed between each re-send. > >>>>> > >>>>> ___ You have failed to adequately and individually address all of the > >>>>> comments and change requests that were proposed in the > >>>>> initial > >> review. > >>>>> ___ You have a misconfigured ~/.hgrc file (i.e. username, email > >>>>> etc) > >>>>> > >>>>> ___ Your computer have a badly configured date and time; confusing > >> the > >>>>> the threaded patch review. > >>>>> > >>>>> ___ Your changes affect IPC mechanism, and you don't present any > >> results > >>>>> for in-service upgradability test. > >>>>> > >>>>> ___ Your changes affect user manual and documentation, your patch > >> series > >>>>> do not contain the patch that updates the Doxygen manual. > >>>>> > >>>>> > >>>>> ------------------------------------------------------------------ > >>>>> ---- > >>>>> -------- This SF.net email is sponsored by Windows: > >>>>> > >>>>> Build for Windows Store. > >>>>> > >>>>> http://p.sf.net/sfu/windows-dev2dev > >>>>> _______________________________________________ > >>>>> Opensaf-devel mailing list > >>>>> [email protected] > >>>>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel > >>>>> > >>>>> > > ---------------------------------------------------------------------- > > -------- This SF.net email is sponsored by Windows: > > > > Build for Windows Store. > > > > http://p.sf.net/sfu/windows-dev2dev > > _______________________________________________ > > Opensaf-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
