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.8 required=3.0 tests=BAYES_00, 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 C3F8BC388F9 for ; Sun, 1 Nov 2020 21:47:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 942E12225C for ; Sun, 1 Nov 2020 21:47:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727335AbgKAVnR (ORCPT ); Sun, 1 Nov 2020 16:43:17 -0500 Received: from smtprelay0207.hostedemail.com ([216.40.44.207]:45618 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727056AbgKAVnQ (ORCPT ); Sun, 1 Nov 2020 16:43:16 -0500 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay06.hostedemail.com (Postfix) with ESMTP id 77864180078BA; Sun, 1 Nov 2020 21:43:15 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: car71_170a629272aa X-Filterd-Recvd-Size: 3553 Received: from XPS-9350.home (unknown [47.151.133.149]) (Authenticated sender: joe@perches.com) by omf10.hostedemail.com (Postfix) with ESMTPA; Sun, 1 Nov 2020 21:43:14 +0000 (UTC) Message-ID: Subject: Re: [PATCH 4/5] mm: shmem: Convert shmem_enabled_show to use sysfs_emit_at From: Joe Perches To: Matthew Wilcox Cc: Hugh Dickins , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Date: Sun, 01 Nov 2020 13:43:13 -0800 In-Reply-To: <20201101211910.GG27442@casper.infradead.org> References: <20201101204834.GF27442@casper.infradead.org> <616b92af9378e9f9697555074bba1e377450477f.camel@perches.com> <20201101211910.GG27442@casper.infradead.org> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.38.1-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 2020-11-01 at 21:19 +0000, Matthew Wilcox wrote: > 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) > > > >   > > > > > > > >  #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_SYSFS) > > > >  static ssize_t shmem_enabled_show(struct kobject *kobj, > > > > - struct kobj_attribute *attr, char *buf) > > > > + struct kobj_attribute *attr, char *buf) > > > >  { > > > >   static const int values[] = { > > > >   SHMEM_HUGE_ALWAYS, > > > > > > Why? > > > > why what? > > Why did you change this? Are you asking about the function argument alignment or the commit message? The function argument alignment because the function is being updated. The commit itself because sysfs_emit is the documented preferred interface. > > > > @@ -4034,16 +4034,19 @@ static ssize_t shmem_enabled_show(struct kobject *kobj, > > > >   SHMEM_HUGE_DENY, > > > >   SHMEM_HUGE_FORCE, > > > >   }; > > > > - int i, count; > > > > - > > > > - for (i = 0, count = 0; i < ARRAY_SIZE(values); i++) { > > > > - const char *fmt = shmem_huge == values[i] ? "[%s] " : "%s "; > > > > + int len = 0; > > > > + int i; > > > > > > Better: > > > int i, len = 0; > > > > I generally disagree as I think it better to have each declaration on an > > individual line. > > You're wrong. I'm not wrong. We just disagree. Look for yourself at typical declaration use in the kernel. The typical style is single line declarations. That single declaration per line style is also documented in coding-style.rst "To this end, use just one data declaration per line (no commas for multiple data declarations)" > Look, this isn't performance sensitive code. Just do something simple. > > if (shmem_huge == values[i]) > buf += sysfs_emit(buf, "[%s]", > shmem_format_huge(values[i])); > else > buf += sysfs_emit(buf, "%s", > shmem_format_huge(values[i])); > if (i == ARRAY_SIZE(values) - 1) > buf += sysfs_emit(buf, "\n"); > else > buf += sysfs_emit(buf, " "); > > Shame there's no sysfs_emitc, but there you go. I think what's there is simple. And your suggested code doesn't work. sysfs_emit is used for single emits. sysfs_emit_at is used for multiple emits.