On Mon, May 08, 2017 at 09:41:08AM +0200, Luca Abeni wrote: > Hi Peter, > > sorry for the delay; anyway, I am working on fixing the patchset > according to the comments I received.... > > When working on one of your comments, I have a doubt: > > On Mon, 27 Mar 2017 16:26:33 +0200 > Peter Zijlstra <[email protected]> wrote: > [...] > > > > > > #define BW_SHIFT 20 > > #define BW_UNIT (1 << BW_SHIFT) > > > > static inline > > u64 grub_reclaim(u64 delta, struct rq *rq, struct sched_dl_entity > > *dl_se) { > > u64 u_inact = rq->dl.this_bw - rq->dl.running_bw; /* Utot - > > Uact */ u64 u_act; > [...] > > I think introducing the BW_SHIFT and BW_UNIT defines can be more useful > in a previous patch (patch 4, where I introduce the "grub_reclaim()" > function, and use ">> 20" for the first time.
Sure.. > Moreover, the "20" magic number is already used in core.c... Should I > introduce the defines in sched/sched.h, and change the existing core.c > code too? Yes please. > Is it ok to embed this change in patch 4 (sched/deadline: > implement GRUB accounting), or should it go in a separate patch? Whatever you feel is nicest. Currently the thing is fully contained in the one to_ratio() function (afaict), so the first patch where you make it escape would be fine.
