ata: Remove inline attribute from ata_sff_host_intr()

PATCH In review David Daney 3 participants 5 messages Nov 21, 2008 – Nov 24, 2008 libata

lore

Share & tools

lore

David Daney

author

2008-11-21 00:39 UTC #0 lore raw

PATCH 1/1 ata: Remove inline attribute from ata_sff_host_intr()

ata: Remove inline attribute from ata_sff_host_intr()

ata_sff_host_intr is a public function, it should not be declared inline.

Signed-off-by David Daney <[email protected]>

---
drivers/ata/libata-sff.c | 4 ++--

1 files changed, 2 insertions(+), 2 deletions(-)

drivers/ata/libata-sff.c +2 -2

index 9033d16..73d7dbb 100644
@@ -1542,8 +1542,8 @@ bool ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc)
 *	RETURNS:
 *	One if interrupt was handled, zero if not (shared irq).
 */
-inline unsigned int ata_sff_host_intr(struct ata_port *ap,
-				      struct ata_queued_cmd *qc)
+unsigned int ata_sff_host_intr(struct ata_port *ap,
+			       struct ata_queued_cmd *qc)

{
	struct ata_eh_info *ehi = &ap->link.eh_info;
	u8 status, host_stat = 0;

Tejun Heo

2008-11-24 05:20 UTC #1 lore raw

Quoted 3 lines · David Daney

ata: Remove inline attribute from ata_sff_host_intr()
ata_sff_host_intr is a public function, it should not be declared inline.

Why not?

Signature · 1 line

tejun

Robert Hancock

2008-11-24 05:42 UTC #2 lore raw

Quoted 7 lines · Tejun Heo

Quoted 3 lines · David Daney
ata: Remove inline attribute from ata_sff_host_intr()
ata_sff_host_intr is a public function, it should not be declared inline.
Why not?

Well, it's a bit unusual to have a non-static inline (an exported symbol no less) in a C file.. how could any callers outside the file ever actually inline it? Aside from that it seems a bit big for inlining..

Tejun Heo

2008-11-24 05:49 UTC #3 lore raw

Quoted 7 lines · Robert Hancock

Quoted 6 lines · Tejun Heo
Quoted 4 lines · David Daney
ata: Remove inline attribute from ata_sff_host_intr()
ata_sff_host_intr is a public function, it should not be declared inline.

Quoted 6 lines

Quoted 2 lines
Why not?
Well, it's a bit unusual to have a non-static inline (an exported symbol no less) in a C file.. how could any callers outside the file ever actually inline it? Aside from that it seems a bit big for inlining..

The goal there is to inline ata_sff_host_intr() into ata_sff_interrupt() and then export a separate copy to other users as ata_sff_interrupt() is very hot. Looking at the disassembly, gcc is doing what it's told to do, so the inline actually is doing something useful there.

Thanks.

Signature · 1 line

tejun

David Daney

author

2008-11-24 17:21 UTC #4 lore raw

Quoted 9 lines · Tejun Heo

Quoted 8 lines · Robert Hancock
Quoted 7 lines · Tejun Heo
Quoted 4 lines · David Daney
ata: Remove inline attribute from ata_sff_host_intr()
ata_sff_host_intr is a public function, it should not be declared inline.
Why not?

Quoted 9 lines

Quoted 3 lines
Well, it's a bit unusual to have a non-static inline (an exported symbol no less) in a C file.. how could any callers outside the file ever actually inline it? Aside from that it seems a bit big for inlining..
The goal there is to inline ata_sff_host_intr() into ata_sff_interrupt() and then export a separate copy to other users as ata_sff_interrupt() is very hot. Looking at the disassembly, gcc is doing what it's told to do, so the inline actually is doing something useful there.

Have you actually profiled it both ways?

I was assuming that the function used to be 'static inline' and that the inline was some sort of typo left over from when static might have been removed.

In any event, do with it what you wish, it just looked weird to me.

David Daney