* [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock()
@ 2010-06-12 2:34 Divyesh Shah
2010-06-12 2:35 ` [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats Divyesh Shah
2010-06-12 7:19 ` [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Ingo Molnar
0 siblings, 2 replies; 7+ messages in thread
From: Divyesh Shah @ 2010-06-12 2:34 UTC (permalink / raw)
To: jaxboe; +Cc: peterz, mingo, piotr, linux-kernel, vgoyal
calls in the block layer. This was a temporary fix added.
Signed-off-by: Divyesh Shah <dpshah@google.com>
---
include/linux/blkdev.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 09a8402..ebe788e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1218,16 +1218,12 @@ int kblockd_schedule_work(struct request_queue *q, struct work_struct *work);
*/
static inline void set_start_time_ns(struct request *req)
{
- preempt_disable();
req->start_time_ns = sched_clock();
- preempt_enable();
}
static inline void set_io_start_time_ns(struct request *req)
{
- preempt_disable();
req->io_start_time_ns = sched_clock();
- preempt_enable();
}
static inline uint64_t rq_start_time_ns(struct request *req)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats.
2010-06-12 2:34 [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Divyesh Shah
@ 2010-06-12 2:35 ` Divyesh Shah
2010-06-12 7:18 ` Ingo Molnar
2010-06-12 7:19 ` [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Ingo Molnar
1 sibling, 1 reply; 7+ messages in thread
From: Divyesh Shah @ 2010-06-12 2:35 UTC (permalink / raw)
To: jaxboe; +Cc: peterz, mingo, piotr, linux-kernel, vgoyal
This will take care of the pre-emptive kernel issue and the unbounded
TSC drift problem. We will lose resolution though in some cases.
Signed-off-by: Divyesh Shah <dpshah@google.com>
---
block/blk-cgroup.c | 22 +++++++++++-----------
include/linux/blkdev.h | 4 ++--
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index a680964..711766d 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -135,19 +135,19 @@ static void blkio_set_start_group_wait_time(struct blkio_group *blkg,
return;
if (blkg == curr_blkg)
return;
- blkg->stats.start_group_wait_time = sched_clock();
+ blkg->stats.start_group_wait_time = ktime_to_ns(ktime_get());
blkio_mark_blkg_waiting(&blkg->stats);
}
/* This should be called with the blkg->stats_lock held. */
static void blkio_update_group_wait_time(struct blkio_group_stats *stats)
{
- unsigned long long now;
+ u64 now;
if (!blkio_blkg_waiting(stats))
return;
- now = sched_clock();
+ now = ktime_to_ns(ktime_get());
if (time_after64(now, stats->start_group_wait_time))
stats->group_wait_time += now - stats->start_group_wait_time;
blkio_clear_blkg_waiting(stats);
@@ -156,12 +156,12 @@ static void blkio_update_group_wait_time(struct blkio_group_stats *stats)
/* This should be called with the blkg->stats_lock held. */
static void blkio_end_empty_time(struct blkio_group_stats *stats)
{
- unsigned long long now;
+ u64 now;
if (!blkio_blkg_empty(stats))
return;
- now = sched_clock();
+ now = ktime_to_ns(ktime_get());
if (time_after64(now, stats->start_empty_time))
stats->empty_time += now - stats->start_empty_time;
blkio_clear_blkg_empty(stats);
@@ -173,7 +173,7 @@ void blkiocg_update_set_idle_time_stats(struct blkio_group *blkg)
spin_lock_irqsave(&blkg->stats_lock, flags);
BUG_ON(blkio_blkg_idling(&blkg->stats));
- blkg->stats.start_idle_time = sched_clock();
+ blkg->stats.start_idle_time = ktime_to_ns(ktime_get());
blkio_mark_blkg_idling(&blkg->stats);
spin_unlock_irqrestore(&blkg->stats_lock, flags);
}
@@ -182,13 +182,13 @@ EXPORT_SYMBOL_GPL(blkiocg_update_set_idle_time_stats);
void blkiocg_update_idle_time_stats(struct blkio_group *blkg)
{
unsigned long flags;
- unsigned long long now;
+ u64 now;
struct blkio_group_stats *stats;
spin_lock_irqsave(&blkg->stats_lock, flags);
stats = &blkg->stats;
if (blkio_blkg_idling(stats)) {
- now = sched_clock();
+ now = ktime_to_ns(ktime_get());
if (time_after64(now, stats->start_idle_time))
stats->idle_time += now - stats->start_idle_time;
blkio_clear_blkg_idling(stats);
@@ -237,7 +237,7 @@ void blkiocg_set_start_empty_time(struct blkio_group *blkg)
return;
}
- stats->start_empty_time = sched_clock();
+ stats->start_empty_time = ktime_to_ns(ktime_get());
blkio_mark_blkg_empty(stats);
spin_unlock_irqrestore(&blkg->stats_lock, flags);
}
@@ -314,7 +314,7 @@ void blkiocg_update_completion_stats(struct blkio_group *blkg,
{
struct blkio_group_stats *stats;
unsigned long flags;
- unsigned long long now = sched_clock();
+ u64 now = ktime_to_ns(ktime_get());
spin_lock_irqsave(&blkg->stats_lock, flags);
stats = &blkg->stats;
@@ -464,7 +464,7 @@ blkiocg_reset_stats(struct cgroup *cgroup, struct cftype *cftype, u64 val)
int i;
#ifdef CONFIG_DEBUG_BLK_CGROUP
bool idling, waiting, empty;
- unsigned long long now = sched_clock();
+ u64 now = ktime_to_ns(ktime_get());
#endif
blkcg = cgroup_to_blkio_cgroup(cgroup);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ebe788e..f174b34 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1218,12 +1218,12 @@ int kblockd_schedule_work(struct request_queue *q, struct work_struct *work);
*/
static inline void set_start_time_ns(struct request *req)
{
- req->start_time_ns = sched_clock();
+ req->start_time_ns = ktime_to_ns(ktime_get());
}
static inline void set_io_start_time_ns(struct request *req)
{
- req->io_start_time_ns = sched_clock();
+ req->io_start_time_ns = ktime_to_ns(ktime_get());
}
static inline uint64_t rq_start_time_ns(struct request *req)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats.
2010-06-12 2:35 ` [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats Divyesh Shah
@ 2010-06-12 7:18 ` Ingo Molnar
2010-06-12 15:51 ` Divyesh Shah
0 siblings, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2010-06-12 7:18 UTC (permalink / raw)
To: Divyesh Shah; +Cc: jaxboe, peterz, piotr, linux-kernel, vgoyal, Thomas Gleixner
* Divyesh Shah <dpshah@google.com> wrote:
> This will take care of the pre-emptive kernel issue and the unbounded
> TSC drift problem. We will lose resolution though in some cases.
> - blkg->stats.start_group_wait_time = sched_clock();
> + blkg->stats.start_group_wait_time = ktime_to_ns(ktime_get());
Ugh!
ktime_get() can have insanely high overhead. Peter has added local_clock(), if
then you should use that and apply checks to make sure the result isnt
negative.
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock()
2010-06-12 2:34 [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Divyesh Shah
2010-06-12 2:35 ` [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats Divyesh Shah
@ 2010-06-12 7:19 ` Ingo Molnar
2010-06-12 15:52 ` Divyesh Shah
1 sibling, 1 reply; 7+ messages in thread
From: Ingo Molnar @ 2010-06-12 7:19 UTC (permalink / raw)
To: Divyesh Shah; +Cc: jaxboe, peterz, piotr, linux-kernel, vgoyal, Thomas Gleixner
* Divyesh Shah <dpshah@google.com> wrote:
> calls in the block layer. This was a temporary fix added.
Not good, this commit reintroduces those ugly warnings, if someone happens to
bisect straight into this commit.
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats.
2010-06-12 7:18 ` Ingo Molnar
@ 2010-06-12 15:51 ` Divyesh Shah
0 siblings, 0 replies; 7+ messages in thread
From: Divyesh Shah @ 2010-06-12 15:51 UTC (permalink / raw)
To: Ingo Molnar; +Cc: jaxboe, peterz, piotr, linux-kernel, vgoyal, Thomas Gleixner
On Sat, Jun 12, 2010 at 12:18 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Divyesh Shah <dpshah@google.com> wrote:
>
>> This will take care of the pre-emptive kernel issue and the unbounded
>> TSC drift problem. We will lose resolution though in some cases.
>
>> - blkg->stats.start_group_wait_time = sched_clock();
>> + blkg->stats.start_group_wait_time = ktime_to_ns(ktime_get());
>
> Ugh!
>
> ktime_get() can have insanely high overhead. Peter has added local_clock(), if
> then you should use that and apply checks to make sure the result isnt
> negative.
Ok. The negative checks are already in the code.
>
> Ingo
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock()
2010-06-12 7:19 ` [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Ingo Molnar
@ 2010-06-12 15:52 ` Divyesh Shah
2010-06-13 10:01 ` Ingo Molnar
0 siblings, 1 reply; 7+ messages in thread
From: Divyesh Shah @ 2010-06-12 15:52 UTC (permalink / raw)
To: Ingo Molnar; +Cc: jaxboe, peterz, piotr, linux-kernel, vgoyal, Thomas Gleixner
On Sat, Jun 12, 2010 at 12:19 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Divyesh Shah <dpshah@google.com> wrote:
>
>> calls in the block layer. This was a temporary fix added.
>
> Not good, this commit reintroduces those ugly warnings, if someone happens to
> bisect straight into this commit.
You're right. In the next version, I'll merge both patches into one so
this cannot happen.
>
> Ingo
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock()
2010-06-12 15:52 ` Divyesh Shah
@ 2010-06-13 10:01 ` Ingo Molnar
0 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2010-06-13 10:01 UTC (permalink / raw)
To: Divyesh Shah; +Cc: jaxboe, peterz, piotr, linux-kernel, vgoyal, Thomas Gleixner
* Divyesh Shah <dpshah@google.com> wrote:
> On Sat, Jun 12, 2010 at 12:19 AM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Divyesh Shah <dpshah@google.com> wrote:
> >
> >> calls in the block layer. This was a temporary fix added.
> >
> > Not good, this commit reintroduces those ugly warnings, if someone happens to
> > bisect straight into this commit.
>
> You're right. In the next version, I'll merge both patches into one so
> this cannot happen.
Yeah - or if you really want to make a point of doing the two things in two
separate patches you can first fix the API, _then_ remove the preemption
enable/disable.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-06-13 10:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-12 2:34 [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Divyesh Shah
2010-06-12 2:35 ` [PATCH 2/2] Use ktime_get() instead of sched_clock() for blkio cgroup stats Divyesh Shah
2010-06-12 7:18 ` Ingo Molnar
2010-06-12 15:51 ` Divyesh Shah
2010-06-12 7:19 ` [PATCH 1/2] Remove preempt_enable/disable calls around sched_clock() Ingo Molnar
2010-06-12 15:52 ` Divyesh Shah
2010-06-13 10:01 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).