On Fri 21 Oct 2016 06:21:59 PM CEST, Pradeep Jagadeesh
<pradeepkiruv...@gmail.com> wrote:
+static bool fsdev_throttle_check_for_wait(FsThrottle *fst, bool is_write)
+{
+ return throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
^
There's an extra whitespace there.
Removed
+static void fsdev_throttle_schedule_next_request(FsThrottle *fst, bool
is_write)
+{
+ if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+ if (fsdev_throttle_check_for_wait(fst, is_write)) {
+ return;
+ }
+ }
+ qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+}
That is wrong, you're calling qemu_co_queue_next() even if the queue is
empty. That line should be inside the 'if' block.
Done!
I still think that -after the changes you made in v6- you could put the
contents of that whole function inside fsdev_co_throttle_request(), and
at the same time get rid of fsdev_throttle_check_for_wait() and call
throttle_schedule_timer() directly. The resulting code will be smaller
and probably easier to read. But those are just style decisions.
I just would like to keep these functions.
+static int get_num_bytes(struct iovec *iov, int iovcnt)
+{
+ int i;
+ int bytes = 0;
+
+ for (i = 0; i < iovcnt; i++) {
+ bytes += iov[i].iov_len;
+ }
+ return bytes;
+}
I overlooked that in my previous review, but 'bytes' should be unsigned
(both here and in fsdev_co_throttle_request).
Done!
I still have to review the init/cleanup functions (I'll try to do it on
monday) but else the code looks fine now.
Ok, I am out of office for whole week. I can send the next patch only on
next saturday/sunday. Please let me know all your comments by then.
-Pradeep
Berto