Re: [Freeipmi-devel] KCS Driver & SMS_ATN Register

[Al Chu]

Hi Matt,

On Thu, 2010-02-25 at 08:25 -0800, Matt Jerdonek wrote:
> Al,
> 
> I'm not sure I understand your concern.  Using either the blocking
> semaphore as I suggested, or the blocking polling as you suggested,
> the application will have to create at least 2 threads: one to wait
> for events, and another to do everything else.  Right?

I don't see it that way.  I could see someone programming a single
thread and only wanting to poll the SMS_ATN bit, and process events as
they occur.  Not doing any other KCS. e.g.

main()
{
   setup_kcs();

   while (1) {
      kcs_wait_for_sms()
      get_message_flags()
      process_event()
   }
}

> If that's the case, isn't using a semaphore a better approach in that
> it doesn't use as many processor cycles and alerts the application
> immediately?

Maybe I didn't describe it well.  The concern I have with your patch (if
I'm reading it correctly, correct me if I'm wrong) is that the only time
the SMS ATN bit is checked is in _ipmi_kcs_get_status().
_ipmi_kcs_get_status() will only be called through other KCS functions
like ipmi_kcs_read() and ipmi_kcs_write().

So in order for the SMS ATN bit to be checked, ipmi_kcs_read() and
ipmi_kcs_write() have to be called, either by your application or other
IPMI going on in the system, otherwise the SMS_ATN bit will never be
checked.  Correct?  Under your patch, in the above code snippet,
kcs_wait_for_sms() will never return, b/c no other KCS calls are going
on (unless they are other KCS IPMI going on in the system elsewhere). 

Perhaps within your patch, you assumed other IPMI going on in other
parts of the system?

Al

> Thanks,
> -Matt
> 
> 
> 
> ______________________________________________________________________
> From: Al Chu <address@hidden>
> To: Matt Jerdonek <address@hidden>
> Cc: Anand Babu Periasamy <address@hidden>; address@hidden
> Sent: Wed, February 24, 2010 12:49:59 PM
> Subject: Re: [Freeipmi-devel] KCS Driver & SMS_ATN Register
> 
> Hey Matt,
> 
> I noticed one or two minor nits, but I can fix em.  I guess I am only
> perplexed by this design choice.  It seems you want to have two
> threads.
> One thread does normal IPMI regularly, and the other thread will wait
> for the SMS_ATN bit.  It appears that _ipmi_kcs_get_status() is the
> only
> place that the SMS_ATN bit is checked, so you need to be doing some
> type
> of other KCS in order to ever check for it?
> 
> Perhaps it'd be better to just have a function that regularly polls
> the
> SMS_ATN bit, and if it is true, return to the user??  Perhaps
> something
> like:
> 
> poll_sms_atn(unsigned int poll_interval, unsigned int max_iterations)
> {
>   while (count <= max_iterations)
>   {
>       lock_kcs_semaphore();
>       if (sms_atn bit set)
>           break;
>       unlock_kcs_sempahore();  
>       sleep (poll_interval);
>   }    
>   unlock_kcs_sempahore();          
> }
> 
> ????
> 
> 
> Al
> 
> On Tue, 2010-02-23 at 08:27 -0800, Matt Jerdonek wrote:
> > 
> > 
> > Please give the attached patch a look.  Since you two didn't like
> the
> > idea of a callback, I created an API to wait for an event and a
> second
> > API to cancel the wait.  Basically the application will be
> responsible
> > for creating a thread which invokes the API.  The API will block the
> > application's thread until an event occurs.  The application will be
> > responsible for issuing a GET MESSAGE FLAGS command once the thread
> > unblocks.
> > 
> > I had to use a semaphore to block the thread, so I made some small
> > changes to ipmi-semaphores.c as well.
> > 
> > Note: this compiles, but I didn't try to run it yet.  My customer
> has
> > not yet sent me the hardware with the KCS interface, so I don't have
> > hardware to exercise the code.  
> > 
> > Thanks again for your consideration,
> > -Matt
> > 
> > 
> -- 
> Albert Chu
> address@hidden
> Computer Scientist
> High Performance Systems Division
> Lawrence Livermore National Laboratory
> 
> 
> 
-- 
Albert Chu
address@hidden
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory