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

[MORITA Kazutaka]

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)

Thanks,

Kazutaka