Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
To:     "Tejun Heo" <tj@kernel.org>
Cc:     "Jarkko Sakkinen" <jarkko@kernel.org>, dave.hansen@linux.intel.com,
        linux-kernel@vger.kernel.org, linux-sgx@vger.kernel.org,
        cgroups@vger.kernel.org, "Zefan Li" <lizefan.x@bytedance.com>,
        "Johannes Weiner" <hannes@cmpxchg.org>, vipinsh@google.com,
        kai.huang@intel.com, reinette.chatre@intel.com,
        zhiquan1.li@intel.com, kristen@linux.intel.com
Subject: Re: [PATCH] cgroup/misc: Fix an overflow
References: <20230717184719.85523-1-haitao.huang@linux.intel.com>
 <CU4OCLEHU1S5.359W394902648@seitikki> <ZLWPN_xyGFrqqJkV@slm.duckdns.org>
 <op.178pr1qewjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <op.178te0tbwjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <ZLWmdBfcuPUBtk1K@slm.duckdns.org>
Date:   Mon, 17 Jul 2023 20:11:15 -0500
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From:   "Haitao Huang" <haitao.huang@linux.intel.com>
Organization: Intel
Message-ID: <op.1786w1rxwjvjmi@hhuan26-mobl.amr.corp.intel.com>
In-Reply-To: <ZLWmdBfcuPUBtk1K@slm.duckdns.org>
User-Agent: Opera Mail/1.0 (Win32)
Precedence: bulk

Hi
On Mon, 17 Jul 2023 15:37:08 -0500, Tejun Heo <tj@kernel.org> wrote:

> Hello,
>
> On Mon, Jul 17, 2023 at 03:19:38PM -0500, Haitao Huang wrote:
>> Actually, we are using atomic_long_t for 'current' which is the same  
>> width
>> as long defined by arch/compiler. So new_usage should be long to be
>> consistent?
>
> We can use atomic64_t, right? It's slower on 32bit machines but I think  
> it'd
> be better to guarantee resource counter range than micro-optimizing  
> charge
> operations. None of the current users are hot enough for this to matter  
> and
> if somebody becomes that hot, the difference between atomic_t and  
> atomic64_t
> isn't gonna matter that much. We'd need to batch allocations per-cpu and  
> so
> on.
>
>> ditto for event counter. Only max is plain unsigned long but I think it  
>> is
>> also OK as it only compared with 'current' without any arithmetic ops
>> involved.
>> Did I miss something here?
>
> I'm saying that it'd be better to make everything explicitly 64bit.
>

Thanks for the explanation. I think I got it and sent it as a separate  
patch now just to be sure.
BR
Haitao