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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C3A3FC2D0A3 for ; Sun, 1 Nov 2020 21:19:17 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 009E5207C3 for ; Sun, 1 Nov 2020 21:19:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="ZK2GisT1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 009E5207C3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7F8116B005C; Sun, 1 Nov 2020 16:19:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7AA146B005D; Sun, 1 Nov 2020 16:19:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6E7CC6B0068; Sun, 1 Nov 2020 16:19:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0165.hostedemail.com [216.40.44.165]) by kanga.kvack.org (Postfix) with ESMTP id 589386B005C for ; Sun, 1 Nov 2020 16:19:16 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 176F53626 for ; Sun, 1 Nov 2020 21:19:16 +0000 (UTC) X-FDA: 77437115112.20.cake60_5c0c536272aa Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin20.hostedemail.com (Postfix) with ESMTP id EDF11180C07AF for ; Sun, 1 Nov 2020 21:19:15 +0000 (UTC) X-HE-Tag: cake60_5c0c536272aa X-Filterd-Recvd-Size: 4202 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf15.hostedemail.com (Postfix) with ESMTP for ; Sun, 1 Nov 2020 21:19:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=mHXvI0jUyLtPqLG2ZOT9kseyc1b7gzS+qOQUvgHUGC8=; b=ZK2GisT1dGB0fQt/foU9qw5DOv j9xH2R9iQ8B0KN34XuStdIFPZ/oH8BcDRESWdTrRvNJ8Yvt0v0mJKjiH94xGoqQ/a2BIZlRZ78hE6 IBt6kA8+sh+kouR1qfeDyPz8xIQw4iNet/Xhm4nVW6g+rrMg9Sh4kRv/pd9aG36hjeA4Ty4uGD9Eg SPgdXysioMhcKV/XugpyNK4c6pvQl8ZANhXv/hddw769jH3OvgkncnEveWCsgjplZe3kihQkTbALi dOJFXSfORXcf7xWw2pkJPxHIwUeEFwG7Rf5qC5sX3OWOFvWS3fiPJuVCa8u8CO5O5DIu577SyqBu9 jAWLMNkg==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kZKko-0004ya-BA; Sun, 01 Nov 2020 21:19:10 +0000 Date: Sun, 1 Nov 2020 21:19:10 +0000 From: Matthew Wilcox To: Joe Perches Cc: Hugh Dickins , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/5] mm: shmem: Convert shmem_enabled_show to use sysfs_emit_at Message-ID: <20201101211910.GG27442@casper.infradead.org> References: <20201101204834.GF27442@casper.infradead.org> <616b92af9378e9f9697555074bba1e377450477f.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <616b92af9378e9f9697555074bba1e377450477f.camel@perches.com> Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, Nov 01, 2020 at 01:04:35PM -0800, Joe Perches wrote: > On Sun, 2020-11-01 at 20:48 +0000, Matthew Wilcox wrote: > > On Sun, Nov 01, 2020 at 12:12:51PM -0800, Joe Perches wrote: > > > @@ -4024,7 +4024,7 @@ int __init shmem_init(void) > > > =A0 > > >=20 > > > =A0#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_SYSFS= ) > > > =A0static ssize_t shmem_enabled_show(struct kobject *kobj, > > > - struct kobj_attribute *attr, char *buf) > > > + struct kobj_attribute *attr, char *buf) > > > =A0{ > > > =A0 static const int values[] =3D { > > > =A0 SHMEM_HUGE_ALWAYS, > >=20 > > Why? >=20 > why what? Why did you change this? > > > @@ -4034,16 +4034,19 @@ static ssize_t shmem_enabled_show(struct ko= bject *kobj, > > > =A0 SHMEM_HUGE_DENY, > > > =A0 SHMEM_HUGE_FORCE, > > > =A0 }; > > > - int i, count; > > > - > > > - for (i =3D 0, count =3D 0; i < ARRAY_SIZE(values); i++) { > > > - const char *fmt =3D shmem_huge =3D=3D values[i] ? "[%s] " : "%s = "; > > > + int len =3D 0; > > > + int i; > >=20 > > Better: > > int i, len =3D 0; >=20 > I generally disagree as I think it better to have each declaration on a= n > individual line. You're wrong. > > > - count +=3D sprintf(buf + count, fmt, > > > - shmem_format_huge(values[i])); > > > + for (i =3D 0; i < ARRAY_SIZE(values); i++) { > > > + len +=3D sysfs_emit_at(buf, len, > > > + shmem_huge =3D=3D values[i] ? "%s[%s]" : "%s%s", > > > + i ? " " : "", > > > + shmem_format_huge(values[i])); > >=20 > > This is ... complicated. I thought the point of doing all the sysfs_= emit > > stuff was to simplify things. >=20 > The removal of fmt allows the format and argument to be __printf verifi= ed. > Indirected formats do not generally allow that. >=20 > And using sysfs_emit is not really intended to simplify output code, it= 's > used to make sure there isn't a overflow of the PAGE_SIZE output buf wh= en > using sprintf/snprintf. Look, this isn't performance sensitive code. Just do something simple. if (shmem_huge =3D=3D values[i]) buf +=3D sysfs_emit(buf, "[%s]", shmem_format_huge(values[i])); else buf +=3D sysfs_emit(buf, "%s", shmem_format_huge(values[i])); if (i =3D=3D ARRAY_SIZE(values) - 1) buf +=3D sysfs_emit(buf, "\n"); else buf +=3D sysfs_emit(buf, " "); Shame there's no sysfs_emitc, but there you go.