All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-pci@vger.kernel.org,
	Kelsey Skunberg <kelsey.skunberg@gmail.com>,
	rbilovol@cisco.com, linux-kernel@vger.kernel.org,
	stable <stable@vger.kernel.org>,
	ddutile@redhat.com, ruslan.bilovol@gmail.com,
	Kelsey Skunberg <skunberg.kelsey@gmail.com>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	bodong@mellanox.com
Subject: Re: [Linux-kernel-mentees] [PATCH v2] PCI: sysfs: Change bus_rescan and dev_rescan to rescan
Date: Sun, 29 Mar 2020 09:33:04 +0200	[thread overview]
Message-ID: <20200329073304.GA3911175@kroah.com> (raw)
In-Reply-To: <20200328200633.GA102137@google.com>

On Sat, Mar 28, 2020 at 03:06:33PM -0500, Bjorn Helgaas wrote:
> On Thu, Mar 26, 2020 at 07:35:24AM +0100, Greg KH wrote:
> > On Wed, Mar 25, 2020 at 05:10:33PM -0500, Bjorn Helgaas wrote:
> > > -static DEVICE_ATTR_WO(dev_rescan);
> > > +static struct device_attribute dev_attr_dev_rescan = __ATTR(rescan, 0200, NULL,
> > > +							    dev_rescan_store);
> > 
> > Oops, this should just be DEVICE_ATTR(), no need for __ATTR() as this
> > isn't a kobject-only file.
> > 
> > So how about:
> > 
> > static DEVICE_ATTR(rescan, 0200, NULL, dev_rescan_store);
> 
> I don' think DEVICE_ATTR() works in this case because it uses the
> first argument ("rescan") to build both the C symbol for the
> device_attribute struct and the sysfs filename.
> 
> There are two instances in this file.  The two sysfs "rescan" files
> are not a problem, but the two "dev_attr_rescan_name" C symbols *are*.
> We could resolve that by putting the bus attributes in a different
> source file than the dev attributes, but it doesn't seem worth it now.
> 
> I tentatively have the patch below on pci/misc.  I dropped the
> tested-by and reviewed-by because I didn't want to put words in your
> mouths :)

Ah, yeah, you are right __ATTR() is what you need to use here.

Your patch looks good:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <greg@kroah.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Kelsey Skunberg <skunberg.kelsey@gmail.com>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	Kelsey Skunberg <kelsey.skunberg@gmail.com>,
	rbilovol@cisco.com, stable <stable@vger.kernel.org>,
	ddutile@redhat.com, ruslan.bilovol@gmail.com,
	linux-kernel-mentees@lists.linuxfoundation.org,
	bodong@mellanox.com
Subject: Re: [Linux-kernel-mentees] [PATCH v2] PCI: sysfs: Change bus_rescan and dev_rescan to rescan
Date: Sun, 29 Mar 2020 09:33:04 +0200	[thread overview]
Message-ID: <20200329073304.GA3911175@kroah.com> (raw)
In-Reply-To: <20200328200633.GA102137@google.com>

On Sat, Mar 28, 2020 at 03:06:33PM -0500, Bjorn Helgaas wrote:
> On Thu, Mar 26, 2020 at 07:35:24AM +0100, Greg KH wrote:
> > On Wed, Mar 25, 2020 at 05:10:33PM -0500, Bjorn Helgaas wrote:
> > > -static DEVICE_ATTR_WO(dev_rescan);
> > > +static struct device_attribute dev_attr_dev_rescan = __ATTR(rescan, 0200, NULL,
> > > +							    dev_rescan_store);
> > 
> > Oops, this should just be DEVICE_ATTR(), no need for __ATTR() as this
> > isn't a kobject-only file.
> > 
> > So how about:
> > 
> > static DEVICE_ATTR(rescan, 0200, NULL, dev_rescan_store);
> 
> I don' think DEVICE_ATTR() works in this case because it uses the
> first argument ("rescan") to build both the C symbol for the
> device_attribute struct and the sysfs filename.
> 
> There are two instances in this file.  The two sysfs "rescan" files
> are not a problem, but the two "dev_attr_rescan_name" C symbols *are*.
> We could resolve that by putting the bus attributes in a different
> source file than the dev attributes, but it doesn't seem worth it now.
> 
> I tentatively have the patch below on pci/misc.  I dropped the
> tested-by and reviewed-by because I didn't want to put words in your
> mouths :)

Ah, yeah, you are right __ATTR() is what you need to use here.

Your patch looks good:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

  reply	other threads:[~2020-03-29  7:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 15:17 [Linux-kernel-mentees] [PATCH v2] PCI: sysfs: Change bus_rescan and dev_rescan to rescan Kelsey Skunberg
2020-03-25 15:17 ` Kelsey Skunberg
2020-03-25 16:31 ` [Linux-kernel-mentees] " Ruslan Bilovol via Linux-kernel-mentees
2020-03-25 16:31   ` Ruslan Bilovol
2020-03-25 22:10 ` [Linux-kernel-mentees] " Bjorn Helgaas
2020-03-25 22:10   ` Bjorn Helgaas
2020-03-26  6:29   ` Kelsey
2020-03-26  6:29     ` Kelsey
2020-03-28 19:59     ` Bjorn Helgaas
2020-03-28 19:59       ` Bjorn Helgaas
2020-03-29  7:20       ` Kelsey
2020-03-29  7:20         ` Kelsey
2020-03-30 13:09       ` Don Dutile
2020-03-30 13:09         ` Don Dutile
2020-03-26  6:35   ` Greg KH
2020-03-26  6:35     ` Greg KH
2020-03-28 20:06     ` Bjorn Helgaas
2020-03-28 20:06       ` Bjorn Helgaas
2020-03-29  7:33       ` Greg KH [this message]
2020-03-29  7:33         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200329073304.GA3911175@kroah.com \
    --to=greg@kroah.com \
    --cc=bodong@mellanox.com \
    --cc=ddutile@redhat.com \
    --cc=helgaas@kernel.org \
    --cc=kelsey.skunberg@gmail.com \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rbilovol@cisco.com \
    --cc=ruslan.bilovol@gmail.com \
    --cc=skunberg.kelsey@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.