Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
On 08/22/2013 06:10 AM, Andrew Morton wrote: > On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang wrote: > >> The member variable 'delays' may be NULL, so need check NULL before use >> it for all extern functions, just like __delayacct_tsk_init() and >> __delayacct_add_tsk() have already done. > > If task.delays is NULL, the kernel will oops. I don't recall seeing > such oops reports hence I suspect that you missed some check somewhere. > > For example, delayacct_blkio_start() checks current->delays before > calling __delayacct_blkio_start(), so why retest current->delays in > __delayacct_blkio_start()? > > Hmm... Can we assume "__*" extern function is treated as "should not be used by outside" ? If so, the original implementation have done (check NULL before use it for all extern functions), just like your valuable example, thanks. Hmm... and better to remove the useless code in __delayacct_add_tsk(), which the related wrapper function has done, the diff is below: --diff begin diff --git a/kernel/delayacct.c b/kernel/delayacct.c index d473988..da8b153 100644 --- a/kernel/delayacct.c +++ b/kernel/delayacct.c @@ -108,12 +108,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk) struct timespec ts; cputime_t utime, stime, stimescaled, utimescaled; - /* Though tsk->delays accessed later, early exit avoids -* unnecessary returning of other data -*/ - if (!tsk->delays) - goto done; - tmp = (s64)d->cpu_run_real_total; task_cputime(tsk, , ); cputime_to_timespec(utime + stime, ); --diff end-- If this diff is OK, I will send related patch for it. Thanks. -- Chen Gang -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang wrote: > The member variable 'delays' may be NULL, so need check NULL before use > it for all extern functions, just like __delayacct_tsk_init() and > __delayacct_add_tsk() have already done. If task.delays is NULL, the kernel will oops. I don't recall seeing such oops reports hence I suspect that you missed some check somewhere. For example, delayacct_blkio_start() checks current->delays before calling __delayacct_blkio_start(), so why retest current->delays in __delayacct_blkio_start()? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang gang.c...@asianux.com wrote: The member variable 'delays' may be NULL, so need check NULL before use it for all extern functions, just like __delayacct_tsk_init() and __delayacct_add_tsk() have already done. If task.delays is NULL, the kernel will oops. I don't recall seeing such oops reports hence I suspect that you missed some check somewhere. For example, delayacct_blkio_start() checks current-delays before calling __delayacct_blkio_start(), so why retest current-delays in __delayacct_blkio_start()? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
On 08/22/2013 06:10 AM, Andrew Morton wrote: On Tue, 20 Aug 2013 10:40:03 +0800 Chen Gang gang.c...@asianux.com wrote: The member variable 'delays' may be NULL, so need check NULL before use it for all extern functions, just like __delayacct_tsk_init() and __delayacct_add_tsk() have already done. If task.delays is NULL, the kernel will oops. I don't recall seeing such oops reports hence I suspect that you missed some check somewhere. For example, delayacct_blkio_start() checks current-delays before calling __delayacct_blkio_start(), so why retest current-delays in __delayacct_blkio_start()? Hmm... Can we assume __* extern function is treated as should not be used by outside ? If so, the original implementation have done (check NULL before use it for all extern functions), just like your valuable example, thanks. Hmm... and better to remove the useless code in __delayacct_add_tsk(), which the related wrapper function has done, the diff is below: --diff begin diff --git a/kernel/delayacct.c b/kernel/delayacct.c index d473988..da8b153 100644 --- a/kernel/delayacct.c +++ b/kernel/delayacct.c @@ -108,12 +108,6 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk) struct timespec ts; cputime_t utime, stime, stimescaled, utimescaled; - /* Though tsk-delays accessed later, early exit avoids -* unnecessary returning of other data -*/ - if (!tsk-delays) - goto done; - tmp = (s64)d-cpu_run_real_total; task_cputime(tsk, utime, stime); cputime_to_timespec(utime + stime, ts); --diff end-- If this diff is OK, I will send related patch for it. Thanks. -- Chen Gang -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
The member variable 'delays' may be NULL, so need check NULL before use
it for all extern functions, just like __delayacct_tsk_init() and
__delayacct_add_tsk() have already done.
Signed-off-by: Chen Gang
---
kernel/delayacct.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index d473988..10cddce 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -81,11 +81,15 @@ static void delayacct_end(struct timespec *start, struct
timespec *end,
void __delayacct_blkio_start(void)
{
+ if (!current->delays)
+ return;
delayacct_start(>delays->blkio_start);
}
void __delayacct_blkio_end(void)
{
+ if (!current->delays)
+ return;
if (current->delays->flags & DELAYACCT_PF_SWAPIN)
/* Swapin block I/O */
delayacct_end(>delays->blkio_start,
@@ -167,6 +171,8 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
__u64 ret;
unsigned long flags;
+ if (!tsk->delays)
+ return 0;
spin_lock_irqsave(>delays->lock, flags);
ret = nsec_to_clock_t(tsk->delays->blkio_delay +
tsk->delays->swapin_delay);
@@ -176,11 +182,15 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
void __delayacct_freepages_start(void)
{
+ if (!current->delays)
+ return;
delayacct_start(>delays->freepages_start);
}
void __delayacct_freepages_end(void)
{
+ if (!current->delays)
+ return;
delayacct_end(>delays->freepages_start,
>delays->freepages_end,
>delays->freepages_delay,
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kernel/delayacct.c: check NULL for member variable 'delays' in all extern functions.
The member variable 'delays' may be NULL, so need check NULL before use
it for all extern functions, just like __delayacct_tsk_init() and
__delayacct_add_tsk() have already done.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
kernel/delayacct.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index d473988..10cddce 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -81,11 +81,15 @@ static void delayacct_end(struct timespec *start, struct
timespec *end,
void __delayacct_blkio_start(void)
{
+ if (!current-delays)
+ return;
delayacct_start(current-delays-blkio_start);
}
void __delayacct_blkio_end(void)
{
+ if (!current-delays)
+ return;
if (current-delays-flags DELAYACCT_PF_SWAPIN)
/* Swapin block I/O */
delayacct_end(current-delays-blkio_start,
@@ -167,6 +171,8 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
__u64 ret;
unsigned long flags;
+ if (!tsk-delays)
+ return 0;
spin_lock_irqsave(tsk-delays-lock, flags);
ret = nsec_to_clock_t(tsk-delays-blkio_delay +
tsk-delays-swapin_delay);
@@ -176,11 +182,15 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
void __delayacct_freepages_start(void)
{
+ if (!current-delays)
+ return;
delayacct_start(current-delays-freepages_start);
}
void __delayacct_freepages_end(void)
{
+ if (!current-delays)
+ return;
delayacct_end(current-delays-freepages_start,
current-delays-freepages_end,
current-delays-freepages_delay,
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
