linux-kernel - Re: [PATCH 2/3] delayacct: convert task->delays to a object

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1258877-4d02-67d9-a2db-d510eadf8441@gmail.com>
Date:   Mon, 9 Oct 2023 19:38:58 +0800
From:   brookxu <brookxu.cn@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     bsingharora@...il.com, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] delayacct: convert task->delays to a object


在 2023/10/9 16:43, Peter Zijlstra 写道:
> On Sun, Oct 08, 2023 at 07:10:01PM +0800, brookxu wrote:
>
>>>> @@ -1331,7 +1332,7 @@ struct task_struct {
>>>>    	struct page_frag		task_frag;
>>>>    #ifdef CONFIG_TASK_DELAY_ACCT
>>>> -	struct task_delay_info		*delays;
>>>> +	struct task_delay_info		delays;
>>>>    #endif
>>> Yeah, no.
>> Yes, this way will increase about 80 bytes for task_struct, about 0.85% of
>> size of task_struct, I think this just like sched_statistics, so that can
>> better support dynamically enable through sysctl.
> But it's 80 bytes 'nobody' will use. And arguably we should do the same
> with schedstats, that's default disabled and again, that's per-task
> storage nobody ever uses.
>
> Per this argument we can grow task_struct indefinitely until it
> collapses in on itself by the sheer weight of it's information density.
> Every additional field will be a smaller fraction of the total.
>
> Yes, it makes it all a little more cumbersome, but we should really not
> burden everybody with the load of some.
>
> Surely there is another solution... ?

Hi, peter:

I found another question when I tried to allocate task_delay_info on 
demand, it is hard for us to

determine whether tsk->delays is NULL due to memory failure or delayacct 
disabled, if due to

memory failure I think we should not try to allocate it again, otherwise 
we may have performance

issues, such as following code. If we limit only try once, the code is 
very trick..


bool delayacct_enter(struct task_struct *tsk)

{
         if (!static_branch_unlikely(&delayacct_key))
                 return false;

         if (tsk->delays && !IS_ERR(tsk->delays))
                 return true;

         if (!cmpxchg(&tsk->delays, NULL, -1))
                 __delayacct_tsk_init(tsk);

         return !IS_ERR_OR_NULL(tsk->delays);

}

static inline void delayacct_blkio_start(void)
{
         if (delayacct_enable(current))
                 __delayacct_blkio_start();
}

static inline void delayacct_blkio_end(struct task_struct *p)
{
         if (delayacct_enable(p))
                 __delayacct_blkio_end(p);
}


do you have any suggestion?