From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163677AbeCBAgQ (ORCPT ); Thu, 1 Mar 2018 19:36:16 -0500 Received: from mail-ot0-f196.google.com ([74.125.82.196]:39550 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1163343AbeCBAgN (ORCPT ); Thu, 1 Mar 2018 19:36:13 -0500 X-Google-Smtp-Source: AG47ELvZUWCpZSguPuIyrZdyNLFEgCBMIpbs51txNGrr7vQtMYouYvFbioRRq+eg1fMVt7nk7m9x16VJ/w9g3XasN6o= MIME-Version: 1.0 In-Reply-To: References: <20180228234006.21093-1-logang@deltatee.com> <20180228234006.21093-3-logang@deltatee.com> <20180301174450.GF13722@bhelgaas-glaptop.roam.corp.google.com> From: Dan Williams Date: Thu, 1 Mar 2018 16:36:12 -0800 Message-ID: Subject: Re: [PATCH v2 02/10] PCI/P2PDMA: Add sysfs group to display p2pmem stats To: Logan Gunthorpe Cc: Bjorn Helgaas , Linux Kernel Mailing List , linux-pci@vger.kernel.org, linux-nvme@lists.infradead.org, linux-rdma , linux-nvdimm , linux-block@vger.kernel.org, Stephen Bates , Christoph Hellwig , Jens Axboe , Keith Busch , Sagi Grimberg , Bjorn Helgaas , Jason Gunthorpe , Max Gurtovoy , =?UTF-8?B?SsOpcsO0bWUgR2xpc3Nl?= , Benjamin Herrenschmidt , Alex Williamson Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 1, 2018 at 4:15 PM, Logan Gunthorpe wrote: > > > On 01/03/18 10:44 AM, Bjorn Helgaas wrote: >> >> I think these two statements are out of order, since the attributes >> dereference pdev->p2pdma. And it looks like you set "error" >> unnecessarily, since you return immediately looking at it. > > > Per the previous series, sysfs_create_group is must_check for some reason. I > had a printk there but you didn't think it was necessary. So assigning it to > error is the only way to squash the warning. Why not fail the setup if the sysfs_create_group() fails? Sure, it may not be strictly required for userspace to have access to these attributes, but it seems hostile that userspace can't make assumptions about the presence of the "p2pmem" directory relative to the capability being setup.