Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752931AbcKRJoK (ORCPT ); Fri, 18 Nov 2016 04:44:10 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33119 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751556AbcKRJoF (ORCPT ); Fri, 18 Nov 2016 04:44:05 -0500 Date: Fri, 18 Nov 2016 10:44:00 +0100 From: Daniel Vetter To: Bjorn Helgaas Cc: Bjorn Helgaas , David Airlie , Thierry Reding , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v2] vgaarb: Use dev_printk() when possible Message-ID: <20161118094400.eguis3nuemh55sau@phenom.ffwll.local> Mail-Followup-To: Bjorn Helgaas , Bjorn Helgaas , David Airlie , Thierry Reding , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org References: <20161117174758.16810.67625.stgit@bhelgaas-glaptop.roam.corp.google.com> <20161117185921.r3fcd4dhfdlahb6b@phenom.ffwll.local> <20161117202059.GA22428@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161117202059.GA22428@bhelgaas-glaptop.roam.corp.google.com> X-Operating-System: Linux phenom 4.6.0-1-amd64 User-Agent: NeoMutt/20161104 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2674 Lines: 72 On Thu, Nov 17, 2016 at 02:20:59PM -0600, Bjorn Helgaas wrote: > On Thu, Nov 17, 2016 at 07:59:21PM +0100, Daniel Vetter wrote: > > On Thu, Nov 17, 2016 at 11:47:58AM -0600, Bjorn Helgaas wrote: > > > Use dev_printk() when possible. This makes messages more consistent with > > > other device-related messages and, in some cases, adds useful information. > > > This changes messages like this: > > > > > > vgaarb: failed to allocate pci device > > > vgaarb: setting as boot device: PCI:0000:01:00.0 > > > vgaarb: device added: PCI:0000:01:00.0,decodes=io+mem,owns=io+mem,locks=none > > > vgaarb: bridge control possible 0000:01:00.0 > > > > > > to this: > > > > > > pci 0000:01:00.0: vgaarb: failed to allocate VGA arbiter data > > > pci 0000:01:00.0: vgaarb: setting as boot VGA device > > > pci 0000:01:00.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none > > > pci 0000:01:00.0: vgaarb: bridge control possible > > > > > > No functional change intended. > > > > Found one more nit below where there was one relevant change that > > shouldn't be here. > > > > @@ -1189,24 +1194,25 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > > > ret_val = -EPROTO; > > > goto done; > > > } > > > - pr_debug("%s ==> %x:%x:%x.%x\n", curr_pos, > > > - domain, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); > > > - > > > pdev = pci_get_domain_bus_and_slot(domain, bus, devfn); > > > - pr_debug("pdev %p\n", pdev); > > > if (!pdev) { > > > - pr_err("invalid PCI address %x:%x:%x\n", > > > - domain, bus, devfn); > > > + pr_err("invalid PCI address %04x:%02x:%02x.%x\n", > > > + domain, bus, PCI_SLOT(devfn), > > > + PCI_FUNC(devfn)); > > > > Userspace-triggerable dmesg spam is imo not good, this needs to stay at > > debug level. > > I did move these around slightly, but I don't *think* I changed > anything from debug to err level. Previously: > > pr_debug("%s ==> %x:%x:%x.%x\n", ...); > pdev = pci_get_domain_bus_and_slot(...); > pr_debug("pdev %p\n", pdev); > if (!pdev) { > pr_err("invalid PCI address %x:%x:%x\n", ...); > } > > after my patch: > > pdev = pci_get_domain_bus_and_slot(...); > if (!pdev) { > pr_err("invalid PCI address %x:%x:%x\n", ...); > } > pr_debug("%s ==> %x:%x:%x.%x pdev %p\n", ...); > > The pr_err() was there before. I'd be glad to change that to a > pr_debug() if you prefer. Oh right, misread the diff. Applied your patch, but a s/pr_err/pr_debug for anything userspace can trigger in the string parsing would be nice. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch