[PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[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.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 drivers/ata/libata-sff.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 9033d16..73d7dbb 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -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;

Re: [PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[Tejun Heo]

David Daney wrote:
> 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?

-- 
tejun

Re: [PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[Robert Hancock]

Tejun Heo wrote:
> David Daney wrote:
>> 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..

Re: [PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[Tejun Heo]

Robert Hancock wrote:
> Tejun Heo wrote:
>> David Daney wrote:
>>> 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..

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.

-- 
tejun

Re: [PATCH] ata: Remove inline attribute from ata_sff_host_intr()

[David Daney]

Tejun Heo wrote:
> Robert Hancock wrote:
>> Tejun Heo wrote:
>>> David Daney wrote:
>>>> 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..
> 
> 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