[sheepdog] [PATCH 1/4] sheep: add preliminary stat function

[Liu Yuan]

On Mon, Aug 26, 2013 at 02:46:24PM +0900, MORITA Kazutaka wrote:
> At Mon, 26 Aug 2013 13:21:18 +0800,
> Liu Yuan wrote:
> > 
> > On Mon, Aug 26, 2013 at 10:39:26AM +0900, Hitoshi Mitake wrote:
> > > At Sun, 25 Aug 2013 20:27:27 +0800,
> > > Liu Yuan wrote:
> > > > 
> > > > Add a new struct sd_stat to monitor the activities of the sheep daemon, which
> > > > are read by the dog. Currently it is very crude and monitors only request.
> > > > 
> > > > Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> > > > ---
> > > >  include/internal_proto.h |   13 +++++++++++++
> > > >  sheep/request.c          |   33 +++++++++++++++++++++++++++++++++
> > > >  sheep/sheep_priv.h       |    1 +
> > > >  3 files changed, 47 insertions(+)
> > > > 
> > > > diff --git a/include/internal_proto.h b/include/internal_proto.h
> > > > index 2c24a52..adb9646 100644
> > > > --- a/include/internal_proto.h
> > > > +++ b/include/internal_proto.h
> > > > @@ -215,4 +215,17 @@ struct object_cache_info {
> > > >  	uint8_t directio;
> > > >  };
> > > >  
> > > > +struct sd_stat {
> > > > +	struct s_request{
> > > > +		uint64_t gway_active_nr; /* nr of running request */
> > > > +		uint64_t peer_active_nr;
> > > > +		uint64_t gway_total_nr; /* Total nr of requests received */
> > > > +		uint64_t peer_total_nr;
> > > > +		uint64_t gway_total_rx; /* Data in */
> > > > +		uint64_t gway_total_tx; /* Data out */
> > > > +		uint64_t peer_total_rx;
> > > > +		uint64_t peer_total_tx;
> > > > +	} r;
> > > > +};
> > > > +
> > > >  #endif /* __INTERNAL_PROTO_H__ */
> > > > diff --git a/sheep/request.c b/sheep/request.c
> > > > index a79f648..112258a 100644
> > > > --- a/sheep/request.c
> > > > +++ b/sheep/request.c
> > > > @@ -315,6 +315,35 @@ static void queue_local_request(struct request *req)
> > > >  	queue_work(sys->io_wqueue, &req->work);
> > > >  }
> > > >  
> > > > +static inline void stat_request_begin(struct request *req)
> > > > +{
> > > > +	struct sd_req *hdr = &req->rq;
> > > > +
> > > > +	if (is_peer_op(req->op)) {
> > > > +		sys->stat.r.peer_total_nr++;
> > > > +		sys->stat.r.peer_active_nr++;
> > > > +		if (hdr->flags & SD_FLAG_CMD_WRITE)
> > > > +			sys->stat.r.peer_total_rx += hdr->data_length;
> > > > +		else
> > > > +			sys->stat.r.peer_total_tx += hdr->data_length;
> > > > +	} else if(is_gateway_op(req->op)) {
> > > > +		sys->stat.r.gway_total_nr++;
> > > > +		sys->stat.r.gway_active_nr++;
> > > > +		if (hdr->flags & SD_FLAG_CMD_WRITE)
> > > > +			sys->stat.r.gway_total_rx += hdr->data_length;
> > > > +		else
> > > > +			sys->stat.r.gway_total_tx += hdr->data_length;
> > > > +	}
> > > > +}
> > > > +
> > > > +static inline void stat_request_end(struct request *req)
> > > > +{
> > > > +	if (is_peer_op(req->op))
> > > > +		sys->stat.r.peer_active_nr--;
> > > > +	else if(is_gateway_op(req->op))
> > > > +		sys->stat.r.gway_active_nr--;
> > > > +}
> > > > +
> > > 
> > > The above two functions should be annotated with main_fn. BTW, why
> > > should they be inline?
> > 
> > I'm wondering, do we need to mark every functions in main thread as main_fn? I
> 
> If you don't guard sys->stat.* with a lock, they must be called in the
> main thread.  I think this is an important case and suggest adding
> main_fn to them.
> 
> > think only some important ones, by the way, if marked as inline, we can't
> > mark them as main_fn.
> 
> Why?  I added main_fn to them but didn't hit any problems.
> 
>   main_fn static inline void stat_request_end(struct request *req)
> 
>   main_fn static inline void stat_request_end(struct request *req)

No problems but if they are inlined after complied, how does trace checker check
them? There are no symbols of them and or instrumented by mcount.

Thanks
Yuan