From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80417CE7AFB for ; Fri, 6 Sep 2024 10:34:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D8FF56B0085; Fri, 6 Sep 2024 06:34:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D3F2D6B0088; Fri, 6 Sep 2024 06:34:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BDFE76B0089; Fri, 6 Sep 2024 06:34:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 9EDF46B0085 for ; Fri, 6 Sep 2024 06:34:18 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 17D0F161246 for ; Fri, 6 Sep 2024 10:34:18 +0000 (UTC) X-FDA: 82533953796.07.02BA5D2 Received: from mail-vs1-f43.google.com (mail-vs1-f43.google.com [209.85.217.43]) by imf14.hostedemail.com (Postfix) with ESMTP id 529AB100018 for ; Fri, 6 Sep 2024 10:34:16 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="elY/lfmP"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.43 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725618776; a=rsa-sha256; cv=none; b=cU3l+wA/5Vn0iHKnZapMBmLotwzSF0dEYxhespwknXID8OnkeOyKy+ptYrqeupHflceasp 7QfOMym8EGO1v+6/Mr0AwYh+YOj9YscekQF5HArzuk6wdNPxnvUIjMtv27uqbnyJmyqUiC 5x7B3X4eplQLLddosONUb4Lpv+GLlRg= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="elY/lfmP"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.43 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725618776; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6c7arQ1vOOTIRZc+s6ypuOjDH2RPL6P4Bh6fBBps064=; b=LdBAslpv4MehLjEirCrM7wyOfLQydgR7qQzJl08218jszNQLIr76gu7ObCqPt3MYqtCi87 NQ4VmgOAs+iayWXGHfFpBDhD5YsQ0nVVJ5LzzqsPX24QckC46o1Qg3we4wAGPsW8HtIVqc UjQYVp5aVAGtHE86+YPI9F4e2AY5fac= Received: by mail-vs1-f43.google.com with SMTP id ada2fe7eead31-49bc12c0041so568194137.0 for ; Fri, 06 Sep 2024 03:34:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725618855; x=1726223655; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=6c7arQ1vOOTIRZc+s6ypuOjDH2RPL6P4Bh6fBBps064=; b=elY/lfmPEf8M0hghM77/TvhyDc9eW42/8svfoFAflW+yq+C8moOouECamMH81bWGCv Sr6K9IT9Ydei2I8I7KYLLVNtzx5wOVrY32YU0f8lGxeHAoGjlGViN6Nba4JfoFAzeymf 36uAYFPYmHTHrs5KREGvyYeNEy66A5dHK9UWJ5X4H8ai7+6wQdmITtrsgHBeih/LlwBN ux794F+qrqNXtvGGYxiOJhfeDJQhvuV3IYvM7NBTof1SSLTtGYyzp1Y5KUWuPL0WX7Oz Q5qEswK/p55Am+UouST0sMXj6vcEr/OCoQShC8m+UZ6QEssL8fRnTwjNuDbUg4ToqGYM 4sXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725618855; x=1726223655; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6c7arQ1vOOTIRZc+s6ypuOjDH2RPL6P4Bh6fBBps064=; b=ii1bjrQcyo5lQjAPDTAny7xetEt3iknwhA+ZeoryYVL5cQZnq0N4yK8BZ6vKULQ97r VcHLnenhlfOPAaxuPjc2Y86zaZh3JjGcUmipn5F3DB3o/9oxfqHwPkNurIjFb2Z0XFTT WBNk3k6vuglOusQTHT2IL1Vd1NsOHp0YZGgY/oeps6P6nu3570UqtvmFKnVPk0LkBJo3 mZXtZN1TEeXRgy4GswUwzLwnQMIY4IKfzKddhGnnSEJ7LkcF/7m7K+nt+RThqAeb1sGy CCNMMV0qgZk/h2P6O8d4Lp88IO2HtekHqxRJpugSFObB/zcc/W828KpfVfiWcPsJ5qM8 yhRA== X-Forwarded-Encrypted: i=1; AJvYcCUVFwFd1jtlyVkHcrIPkjw19bhWpRuhND5i79GuZDnCNrZv1TN2lGUUIvbbR2I3mpAtC9q+1s5MMQ==@kvack.org X-Gm-Message-State: AOJu0YzxqjGMLBC652PxoahcThtkFTwCMZJMRacC61OUQ8TuTbpcYE/B WrkOMt+Hw6CaXLoC9qkG80SC+UhJh//sAr4YnQTh90T3yJMUYRU3J6MVGZ+pYrhf1wiHYwM/4eq G7UyVzg7zdkiFSTCuuTKkszZka9o= X-Google-Smtp-Source: AGHT+IGnRsvdGg2xjXX4/7DRlCP6WawHt3UxzXrnb2+YsvR6JLsq4270qn1DIEklR/jsXdB1Wbi+ozu0GKhg7WGkABU= X-Received: by 2002:a05:6102:292a:b0:498:cdb0:1d03 with SMTP id ada2fe7eead31-49bde263682mr2405667137.23.1725618855133; Fri, 06 Sep 2024 03:34:15 -0700 (PDT) MIME-Version: 1.0 References: <20240906080047.21409-1-hailong.liu@oppo.com> <20240906095554.dkanjjn2yj6z3g3j@oppo.com> In-Reply-To: <20240906095554.dkanjjn2yj6z3g3j@oppo.com> From: Barry Song <21cnbao@gmail.com> Date: Fri, 6 Sep 2024 22:34:00 +1200 Message-ID: Subject: Re: [PATCH] seq_file: replace kzalloc() with kvzalloc() To: Hailong Liu Cc: viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 529AB100018 X-Stat-Signature: fne1aqa3r3mftedcqm7jgwmi3qq3y7ey X-Rspam-User: X-HE-Tag: 1725618856-579635 X-HE-Meta: U2FsdGVkX1/zMq+zHS9oycN+d7AahjjurC5Bq/zz98s8ultuPdnQ3QhDnnm2KgcukpV5UdNG4sVISVjjwnbXIQNAW2voFqB7x3kZsfoQlFUeU52dA/mJ3IlgWXPiKUhH1p07EAw68DGohAZ+rAmeE6F0e8ydfxmdsYNmOMbfgOXoON7H3IReP5iP9oUMeSwwAxyxeaWuJW/kPrY5Syy+UFS3PhJ1ZzUd4Gsq8XQuhS6ZboFy1b6tzwZsftN5/vjb0S+man+4fooqg+8j1H/7YtQxM46idu0l4SPxz6Zf9BB442xLI7ca97aQDm1dwLpno60up1OSisFYok6JPGiJA4jVBZBXivVX+z/cgzPY5nK0qIeBsMMj2SlnAxF/q3nxFRP0ZMMX4VV5V+f1B6kG34B28oUr8ZeeKvM6fMrImN4MEcz7JXwjBc+RVfP4fi1asu3Xbk/79Ow/B8LiF/oCbtYQ/vB9N6KR40Z61D7o3c0xTZO62xIAljR2aJC4+DX/YRUI3/sNb9aWQTf7dpxr8E+oCNtteEG9bXadY7F3lFGzT0bWEeiFn8+xGkFKbu7zRFsFZwDEWIXJJZSuwStO4S3rcD3XN29uqDvPXEn2xTGq8cdCbEd/0T+go3AgReN5IrDS/mUrxfvcZ7KzhtE1c5LxcijOY6ersAqCSEbROIbVh5LP12gEt91kkRW2uTBVdy2PkkohT07eg6iPvZyMe/inohEx/Z1Fk49YOcSSykQOIsgQI8LpjfDusf2i0dckoyGVN/GgynPC/yZOp4j/HY4FhUknzQVodAzXz7ZDwxL6DEdOG+wVlEeU7/z2OUUvxJmqROhoFIApShZ4c09wg1W9E7FL45T5kC2cjXURB+uG5+e2/H0jfqWbJ7ZRU/zFJxe3NnVBfdi5Furyn4X/ENx/6eHhboDzY5u/gTro89ilXQ0TCsMQqJOZrRZR0P5PHnHHtAkOxs8CNRBibfq raEB1jh6 oCJVO2nMlfL4yqOIBqbdOGSKqIyMRSERlhan0vSTlXssAyYy8Vz3kyGASeKJKL0cmio7gP4yOWzyOt59JJy4vNYxikIfbTOgVjZA6JjBRot41VNIklyK2he6DEX4lbJhfDLu4vspfMbRs7T7UlXV2wpHuFVf6R+QlgRZe2bnNf12tW//5YObuPncsWmeU2eevXCFZTRD74Jpy9GDZ6TM5J48d3j6vlu014txlKv9IWfHPkInHSZydNPA3VLNUblLhw7vLd+EMpIcIDxJ+Ou3ykF8msGKJy7xOL/j/DKfSjKH/HHl6X4tmv0PfZHDADkHf+IBov14w3/XYWl4JMmNxYJ4G5Lpc1xzDxJrXPC9BJhewHdK7St+IsoAD+TnmQ7s0xJAOnUAF5rAevc5TOmAHOVavEyd3onVz8Oq2vB5p6SSKLyjVCkQeP6j7htURqDGOFx3WvsfZzC/lX/F8TxcvtCA08g== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Sep 6, 2024 at 9:56=E2=80=AFPM Hailong Liu w= rote: > > On Fri, 06. Sep 21:25, Barry Song wrote: > > On Fri, Sep 6, 2024 at 8:06=E2=80=AFPM Hailong Liu wrote: > > > > > > __seq_open_private() uses kzalloc() to allocate a private buffer. How= ever, > > > the size of the buffer might be greater than order-3, which may cause > > > allocation failure. To address this issue, use kvzalloc instead. > > > > In general, this patch seems sensible, but do we have a specific exampl= e > > of a driver that uses such a large amount of private data? > > Providing a real-world example of a driver with substantial private dat= a could > > make this patch more convincing:-) > > > To be honest, it's a bit embarrassing, but the issue is that my own drive= r > allocates 256K of memory to store data. :) > > Howeve I grep the __seq_open_private in drivers and found > https://elixir.bootlin.com/linux/v6.11-rc5/source/drivers/net/ethernet/ch= elsio/cxgb4/cxgb4_debugfs.c#L3765 > static int ulprx_la_open(struct inode *inode, struct file *file) > { > struct seq_tab *p; > struct adapter *adap =3D inode->i_private; > > p =3D seq_open_tab(file, ULPRX_LA_SIZE, 8 * sizeof(u32), 1, > ulprx_la_show); > > -> seq_open_tab... > -> p =3D __seq_open_private(f, &seq_tab_ops,= sizeof(*p) + rows * width); > -> ULPRX_LA_SIZE * 8 * sizeof(u32) =3D 32 * = 512 =3D 16384 =3D order-2 > -> if system is in highly fragmation, order-2 might allocation failu= re. > if (!p) > return -ENOMEM; > > t4_ulprx_read_la(adap, (u32 *)p->data); > return 0; > } > So IMO this issue might also be encountered in other drivers. > > I should also change the comment in Documentation/filesystems/seq_file.rs= t > ```rst > There is also a wrapper function to seq_open() called seq_open_private().= It > kmallocs a zero filled block of memory and stores a pointer to it in the > private field of the seq_file structure, returning 0 on success. The > block size is specified in a third parameter to the function, e.g.:: That's correct. Additionally, we should update the changelog to specify that 'the buffer size might exceed order-3, potentially causing allocation failures,' as order-2 could also be a concern, using drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c as an instance. > > static int ct_open(struct inode *inode, struct file *file) > { > return seq_open_private(file, &ct_seq_ops, > sizeof(struct mystruct)); > } > ``` > > if the patch be ACKed. I will add this in next version. > not the authority to acknowledge this, but personally, it makes a lot of sense from the memory management perspective. Please feel free to add Reviewed-by: Barry Song If you plan to send a v2 to correct the changelog and documentation. > > > > > > Signed-off-by: Hailong Liu > > > --- > > > fs/seq_file.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/seq_file.c b/fs/seq_file.c > > > index e676c8b0cf5d..cf23143bbb65 100644 > > > --- a/fs/seq_file.c > > > +++ b/fs/seq_file.c > > > @@ -621,7 +621,7 @@ int seq_release_private(struct inode *inode, stru= ct file *file) > > > { > > > struct seq_file *seq =3D file->private_data; > > > > > > - kfree(seq->private); > > > + kvfree(seq->private); > > > seq->private =3D NULL; > > > return seq_release(inode, file); > > > } > > > @@ -634,7 +634,7 @@ void *__seq_open_private(struct file *f, const st= ruct seq_operations *ops, > > > void *private; > > > struct seq_file *seq; > > > > > > - private =3D kzalloc(psize, GFP_KERNEL_ACCOUNT); > > > + private =3D kvzalloc(psize, GFP_KERNEL_ACCOUNT); > > > if (private =3D=3D NULL) > > > goto out; > > > > > > @@ -647,7 +647,7 @@ void *__seq_open_private(struct file *f, const st= ruct seq_operations *ops, > > > return private; > > > > > > out_free: > > > - kfree(private); > > > + kvfree(private); > > > out: > > > return NULL; > > > } > > > -- > > > 2.30.0 > > > > > > > > > > -- > > Help you, Help me, > Hailong. Thanks Barry