Patchwork [080/493] fddi: remove use of __devexit_p

login
register
mail settings
Submitter Bill Pemberton
Date Nov. 19, 2012, 6:20 p.m.
Message ID <1353349642-3677-80-git-send-email-wfp5p@virginia.edu>
Download mbox | patch
Permalink /patch/200098/
State Rejected
Delegated to: David Miller
Headers show

Comments

Bill Pemberton - Nov. 19, 2012, 6:20 p.m.
CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
needed.

Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
Cc: "Maciej W. Rozycki" <macro@linux-mips.org> 
Cc: netdev@vger.kernel.org 
---
 drivers/net/fddi/defxx.c       | 6 +++---
 drivers/net/fddi/skfp/skfddi.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
Maciej W. Rozycki - Nov. 19, 2012, 6:49 p.m.
On Mon, 19 Nov 2012, Bill Pemberton wrote:

> CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> needed.

 Shouldn't this be switching to __exit_p() instead?  Likewise all the 
other changes concerned (i.e. s/__dev\(init\|exit\)/__\1/), for the sake 
of drivers built into the kernel proper (yes, there are people out there 
still doing that).  Am I missing something?

 This change scores my NAK as it stands, until updated accordingly or 
further justified.

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Nov. 19, 2012, 6:57 p.m.
On Mon, 2012-11-19 at 18:49 +0000, Maciej W. Rozycki wrote:
> On Mon, 19 Nov 2012, Bill Pemberton wrote:
> 
> > CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> > needed.
> 
>  Shouldn't this be switching to __exit_p() instead?  Likewise all the 
> other changes concerned (i.e. s/__dev\(init\|exit\)/__\1/), for the sake 
> of drivers built into the kernel proper (yes, there are people out there 
> still doing that).  Am I missing something?

Drivers can be unbound by poking in sysfs.

Ben.

>  This change scores my NAK as it stands, until updated accordingly or 
> further justified.
David Miller - Nov. 19, 2012, 7:08 p.m.
From: "Maciej W. Rozycki" <macro@linux-mips.org>
Date: Mon, 19 Nov 2012 18:49:00 +0000 (GMT)

> On Mon, 19 Nov 2012, Bill Pemberton wrote:
> 
>  This change scores my NAK as it stands, until updated accordingly or 
> further justified.

I also refuse to apply any of these patches, they are rediculous
and also submitted in an inconsiderate and improper manner.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej W. Rozycki - Nov. 19, 2012, 7:16 p.m.
On Mon, 19 Nov 2012, Maciej W. Rozycki wrote:

> > CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> > needed.
> 
>  Shouldn't this be switching to __exit_p() instead?  Likewise all the 
> other changes concerned (i.e. s/__dev\(init\|exit\)/__\1/), for the sake 
> of drivers built into the kernel proper (yes, there are people out there 
> still doing that).  Am I missing something?
> 
>  This change scores my NAK as it stands, until updated accordingly or 
> further justified.

 I have unconfused myself now, so please replace the above with the 
following question: what about configurations (e.g. buses) that not 
support hotplug at all?  For example apart from PCI the defxx driver 
concerned here supports the TURBOchannel bus that by design does not have 
the concept of live option card removal (no such circuitry).  So should 
now the precious memory be wasted on systems that will never ever handle 
hotplug?

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH - Nov. 19, 2012, 7:29 p.m.
On Mon, Nov 19, 2012 at 07:16:44PM +0000, Maciej W. Rozycki wrote:
> On Mon, 19 Nov 2012, Maciej W. Rozycki wrote:
> 
> > > CONFIG_HOTPLUG is going away as an option so __devexit_p is no longer
> > > needed.
> > 
> >  Shouldn't this be switching to __exit_p() instead?  Likewise all the 
> > other changes concerned (i.e. s/__dev\(init\|exit\)/__\1/), for the sake 
> > of drivers built into the kernel proper (yes, there are people out there 
> > still doing that).  Am I missing something?
> > 
> >  This change scores my NAK as it stands, until updated accordingly or 
> > further justified.
> 
>  I have unconfused myself now, so please replace the above with the 
> following question: what about configurations (e.g. buses) that not 
> support hotplug at all?  For example apart from PCI the defxx driver 
> concerned here supports the TURBOchannel bus that by design does not have 
> the concept of live option card removal (no such circuitry).  So should 
> now the precious memory be wasted on systems that will never ever handle 
> hotplug?

CONFIG_HOTPLUG is always enabled now, so that's not an option anymore.
And again, a user can "hot unbind" a driver from a device from
userspace, no matter if the bus physically supports it or not.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt - Nov. 19, 2012, 9:22 p.m.
On Monday 2012-11-19 20:29, Greg KH wrote:
>
>CONFIG_HOTPLUG is always enabled now, so that's not an option anymore.
>And again, a user can "hot unbind" a driver from a device from
>userspace, no matter if the bus physically supports it or not.

Which mailing list was this originally posted on? (I only have 
subscribed to netdev, and google has not indexed 000/493--001/493 yet.)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bill Pemberton - Nov. 19, 2012, 9:30 p.m.
Jan Engelhardt writes:
> 
> On Monday 2012-11-19 20:29, Greg KH wrote:
> >
> >CONFIG_HOTPLUG is always enabled now, so that's not an option anymore.
> >And again, a user can "hot unbind" a driver from a device from
> >userspace, no matter if the bus physically supports it or not.
> 
> Which mailing list was this originally posted on? (I only have 
> subscribed to netdev, and google has not indexed 000/493--001/493 yet.)
> 

The announcement for this patchset was sent to lkml on Friday.  I
originally didn't have the patches themselves Cc'd to the various
subsystems, so no one other than Greg got the patches themselves on
Friday.  I resent them with Cc's today.
Maciej W. Rozycki - Nov. 21, 2012, 11:49 p.m.
Greg,

> >  I have unconfused myself now, so please replace the above with the 
> > following question: what about configurations (e.g. buses) that not 
> > support hotplug at all?  For example apart from PCI the defxx driver 
> > concerned here supports the TURBOchannel bus that by design does not have 
> > the concept of live option card removal (no such circuitry).  So should 
> > now the precious memory be wasted on systems that will never ever handle 
> > hotplug?
> 
> CONFIG_HOTPLUG is always enabled now, so that's not an option anymore.
> And again, a user can "hot unbind" a driver from a device from
> userspace, no matter if the bus physically supports it or not.

 Hmm, what purpose does this serve for devices that cannot be physically 
removed?  If there is none, shouldn't that policy be set by individual 
drivers or platform even?  Even if HOTPLUG as a whole is unconditional (I 
suppose the amount of space core support itself takes is much less to what 
driver code can).

 TURBOchannel, although valid, is an old exotic case that might not be 
worth arguing for, except for purity maybe.  But there are surely many 
contemporary systems out there that are known they are never going to 
support hot device replacement.  Consider most of the embedded systems for 
example, where devices may even physically be cast into a single SOC (with 
no prospect of chipping off any pieces ever ;) ), that certainly could not 
care less of device replacement, but they do care a lot about memory 
consumption.

 Was this implication considered, discussed and diregarded as not 
important enough compared to benefits from hardcoding HOTPLUG support?  
I'm seriously asking for a pointer, not trying to cause any stir-up -- 
regrettably I fail to follow most discussions these days, but I would like 
to know what the background behind this decision was.  Thanks a lot!

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH - Nov. 22, 2012, 12:23 a.m.
On Wed, Nov 21, 2012 at 11:49:50PM +0000, Maciej W. Rozycki wrote:
> Greg,
> 
> > >  I have unconfused myself now, so please replace the above with the 
> > > following question: what about configurations (e.g. buses) that not 
> > > support hotplug at all?  For example apart from PCI the defxx driver 
> > > concerned here supports the TURBOchannel bus that by design does not have 
> > > the concept of live option card removal (no such circuitry).  So should 
> > > now the precious memory be wasted on systems that will never ever handle 
> > > hotplug?
> > 
> > CONFIG_HOTPLUG is always enabled now, so that's not an option anymore.
> > And again, a user can "hot unbind" a driver from a device from
> > userspace, no matter if the bus physically supports it or not.
> 
>  Hmm, what purpose does this serve for devices that cannot be physically 
> removed?  If there is none, shouldn't that policy be set by individual 
> drivers or platform even?  Even if HOTPLUG as a whole is unconditional (I 
> suppose the amount of space core support itself takes is much less to what 
> driver code can).
> 
>  TURBOchannel, although valid, is an old exotic case that might not be 
> worth arguing for, except for purity maybe.  But there are surely many 
> contemporary systems out there that are known they are never going to 
> support hot device replacement.  Consider most of the embedded systems for 
> example, where devices may even physically be cast into a single SOC (with 
> no prospect of chipping off any pieces ever ;) ), that certainly could not 
> care less of device replacement, but they do care a lot about memory 
> consumption.

Even those don't care about less than 5k of memory, do they?

>  Was this implication considered, discussed and diregarded as not 
> important enough compared to benefits from hardcoding HOTPLUG support?  

Yes, I don't know of any modern system that does not enable
CONFIG_HOTPLUG, do you?

> I'm seriously asking for a pointer, not trying to cause any stir-up -- 
> regrettably I fail to follow most discussions these days, but I would like 
> to know what the background behind this decision was.  Thanks a lot!

See Russell's response in this thread for details if you are curious.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maciej W. Rozycki - Nov. 26, 2012, 4:35 a.m.
On Wed, 21 Nov 2012, Greg KH wrote:

> >  TURBOchannel, although valid, is an old exotic case that might not be 
> > worth arguing for, except for purity maybe.  But there are surely many 
> > contemporary systems out there that are known they are never going to 
> > support hot device replacement.  Consider most of the embedded systems for 
> > example, where devices may even physically be cast into a single SOC (with 
> > no prospect of chipping off any pieces ever ;) ), that certainly could not 
> > care less of device replacement, but they do care a lot about memory 
> > consumption.
> 
> Even those don't care about less than 5k of memory, do they?

 I guess you're right.  As long as it's not 5k per driver + 
who_knows_how_much per platform for some generic stuff that is.  Of course 
this is still a waste, but I can accept it as a compromise between the use 
of machine resources and the cost of maintenance.

> >  Was this implication considered, discussed and diregarded as not 
> > important enough compared to benefits from hardcoding HOTPLUG support?  
> 
> Yes, I don't know of any modern system that does not enable
> CONFIG_HOTPLUG, do you?

 I've never enabled it outside x86 to be honest.  None of the platforms I 
care of supports it in hardware.

> > I'm seriously asking for a pointer, not trying to cause any stir-up -- 
> > regrettably I fail to follow most discussions these days, but I would like 
> > to know what the background behind this decision was.  Thanks a lot!
> 
> See Russell's response in this thread for details if you are curious.

 Hmm, not in my inbox for some reason (unless I'm blind), but thanks for 
the pointer, I'll see if I can chase it online in some mailing list 
archive.

  Maciej
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight - Nov. 26, 2012, 9:33 a.m.
> > Even those don't care about less than 5k of memory, do they?
> 
>  I guess you're right.  As long as it's not 5k per driver +
> who_knows_how_much per platform for some generic stuff that is.  Of course
> this is still a waste, but I can accept it as a compromise between the use

If you are worried about that much memory (eg for a small embedded Linux)
I'r worry about the sizes of all the hash tables that keep popping up.
At least some of them really need converting to some kind of tree.

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
index 6695a1d..538f8bb 100644
--- a/drivers/net/fddi/defxx.c
+++ b/drivers/net/fddi/defxx.c
@@ -3637,7 +3637,7 @@  static struct pci_driver dfx_pci_driver = {
 	.name		= "defxx",
 	.id_table	= dfx_pci_table,
 	.probe		= dfx_pci_register,
-	.remove		= __devexit_p(dfx_pci_unregister),
+	.remove		= dfx_pci_unregister,
 };
 
 static __devinit int dfx_pci_register(struct pci_dev *pdev,
@@ -3668,7 +3668,7 @@  static struct eisa_driver dfx_eisa_driver = {
 		.name	= "defxx",
 		.bus	= &eisa_bus_type,
 		.probe	= dfx_dev_register,
-		.remove	= __devexit_p(dfx_dev_unregister),
+		.remove	= dfx_dev_unregister,
 	},
 };
 #endif /* CONFIG_EISA */
@@ -3689,7 +3689,7 @@  static struct tc_driver dfx_tc_driver = {
 		.name	= "defxx",
 		.bus	= &tc_bus_type,
 		.probe	= dfx_dev_register,
-		.remove	= __devexit_p(dfx_dev_unregister),
+		.remove	= dfx_dev_unregister,
 	},
 };
 #endif /* CONFIG_TC */
diff --git a/drivers/net/fddi/skfp/skfddi.c b/drivers/net/fddi/skfp/skfddi.c
index 3d9a459..cbabb74 100644
--- a/drivers/net/fddi/skfp/skfddi.c
+++ b/drivers/net/fddi/skfp/skfddi.c
@@ -2243,7 +2243,7 @@  static struct pci_driver skfddi_pci_driver = {
 	.name		= "skfddi",
 	.id_table	= skfddi_pci_tbl,
 	.probe		= skfp_init_one,
-	.remove		= __devexit_p(skfp_remove_one),
+	.remove		= skfp_remove_one,
 };
 
 static int __init skfd_init(void)