Messages in this thread | | | Date | Wed, 4 Nov 2009 18:59:45 +0100 | Subject | Re: [PATCH 02/20] blkio: Change CFQ to use CFS like queue time stamps | From | Corrado Zoccolo <> |
| |
On Wed, Nov 4, 2009 at 5:37 PM, Vivek Goyal <vgoyal@redhat.com> wrote: > On Wed, Nov 04, 2009 at 09:30:34AM -0500, Jeff Moyer wrote: >> Vivek Goyal <vgoyal@redhat.com> writes: >> > > Thanks for the review Jeff. > >> > o Currently CFQ provides priority scaled time slices to processes. If a process >> > does not use the time slice, either because process did not have sufficient >> > IO to do or because think time of process is large and CFQ decided to disable >> > idling, then processes looses it time slice share. >> ^^^^^^ >> loses >> should be 'process loses'
>> > +static inline u64 max_vdisktime(u64 min_vdisktime, u64 vdisktime) >> > +{ >> > + s64 delta = (s64)(vdisktime - min_vdisktime); >> > + if (delta > 0) >> > + min_vdisktime = vdisktime; >> > + >> > + return min_vdisktime; >> > +} >> > + >> > +static inline u64 min_vdisktime(u64 min_vdisktime, u64 vdisktime) >> > +{ >> > + s64 delta = (s64)(vdisktime - min_vdisktime); >> > + if (delta < 0) >> > + min_vdisktime = vdisktime; >> > + >> > + return min_vdisktime; >> > +} >> >> Is there a reason you've reimplemented min and max? > > I think you are referring to min_t and max_t. Will these macros take care > of wrapping too? > > For example, if I used min_t(u64, A, B), then unsigned comparision will > not work right wrapping has just taken place for any of the A or B. So if > A=-1 and B=2, then min_t() would return B as minimum. This is not right > in our case. > > If we do signed comparison (min_t(s64, A, B)), that also seems to be > broken in another case where a value of variable moves from 63bits to 64bits, > (A=0x7fffffffffffffff, B=0x8000000000000000). Above will return B as minimum but > in our scanario, vdisktime will progress from 0x7fffffffffffffff to > 0x8000000000000000 and A should be returned as minimum (unsigned > comparison). > > Hence I took these difnitions from CFS. If those are times (measured in jiffies), why are you using u64? You could use unsigned long and time_before/time_after, that perform the proper wrap checking.
Corrado -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |