Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1197083 > unrolled thread
| Started by | Murali Karicheri <m-karicheri2@ti.com> |
|---|---|
| First post | 2015-07-31 17:10 +0200 |
| Last post | 2015-08-03 18:00 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) Murali Karicheri <m-karicheri2@ti.com> - 2015-07-31 17:10 +0200
RE: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) David Laight <David.Laight@ACULAB.COM> - 2015-07-31 18:30 +0200
Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) Viresh Kumar <viresh.kumar@linaro.org> - 2015-07-31 18:30 +0200
Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) Murali Karicheri <m-karicheri2@ti.com> - 2015-08-03 18:00 +0200
| From | Murali Karicheri <m-karicheri2@ti.com> |
|---|---|
| Date | 2015-07-31 17:10 +0200 |
| Subject | Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) |
| Message-ID | <pSk6B-Yx-19@gated-at.bofh.it> |
On 07/31/2015 04:38 AM, Viresh Kumar wrote:
> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there
> is no need to do that again from its callers. Drop it.
>
IS_ERR_OR_NULL() is defined as
static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
{
return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}
So the unlikely() applies only to second part. Wouldn't that be a
problem for optimization?
Murali
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/net/ethernet/ti/netcp_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> index ec8ed30196f3..f685a19a3703 100644
> --- a/drivers/net/ethernet/ti/netcp_core.c
> +++ b/drivers/net/ethernet/ti/netcp_core.c
> @@ -1016,7 +1016,7 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
> }
>
> desc = knav_pool_desc_get(netcp->tx_pool);
> - if (unlikely(IS_ERR_OR_NULL(desc))) {
> + if (IS_ERR_OR_NULL(desc)) {
> dev_err(netcp->ndev_dev, "out of TX desc\n");
> dma_unmap_single(dev, dma_addr, pkt_len, DMA_TO_DEVICE);
> return NULL;
> @@ -1049,7 +1049,7 @@ netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
> }
>
> ndesc = knav_pool_desc_get(netcp->tx_pool);
> - if (unlikely(IS_ERR_OR_NULL(ndesc))) {
> + if (IS_ERR_OR_NULL(ndesc)) {
> dev_err(netcp->ndev_dev, "out of TX desc for frags\n");
> dma_unmap_page(dev, dma_addr, buf_len, DMA_TO_DEVICE);
> goto free_descs;
>
--
Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | David Laight <David.Laight@ACULAB.COM> |
|---|---|
| Date | 2015-07-31 18:30 +0200 |
| Subject | RE: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL) |
| Message-ID | <pSlm3-2GV-31@gated-at.bofh.it> |
| In reply to | #1197083 |
From: Murali Karicheri
> Sent: 31 July 2015 16:04
> On 07/31/2015 04:38 AM, Viresh Kumar wrote:
> > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there
> > is no need to do that again from its callers. Drop it.
> >
>
> IS_ERR_OR_NULL() is defined as
>
> static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
> {
> return !ptr || IS_ERR_VALUE((unsigned long)ptr);
> }
>
> So the unlikely() applies only to second part. Wouldn't that be a
> problem for optimization?
Ugg...
The unlikely() in IS_ERR_VALUE() is likely to stop the compiler
doing a single 'window' comparison for the range [-MAX_ERROR .. 0].
So you are likely to end up with 2 comparisons.
I suspect that:
return IS_ERR_VALUE((unsigned long)ptr - 1);
would be a much better test.
(Ignoring the off-by-one for the highest error.)
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Viresh Kumar <viresh.kumar@linaro.org> |
|---|---|
| Date | 2015-07-31 18:30 +0200 |
| Message-ID | <pSlm3-2GV-33@gated-at.bofh.it> |
| In reply to | #1197083 |
On 31-07-15, 11:04, Murali Karicheri wrote:
> On 07/31/2015 04:38 AM, Viresh Kumar wrote:
> >IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there
> >is no need to do that again from its callers. Drop it.
> >
>
> IS_ERR_OR_NULL() is defined as
>
> static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
> {
> return !ptr || IS_ERR_VALUE((unsigned long)ptr);
> }
>
> So the unlikely() applies only to second part. Wouldn't that be a
> problem for optimization?
This is what the first patch of the series does:
http://permalink.gmane.org/gmane.linux.kernel/2009151
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Murali Karicheri <m-karicheri2@ti.com> |
|---|---|
| Date | 2015-08-03 18:00 +0200 |
| Message-ID | <pTqjD-7A6-5@gated-at.bofh.it> |
| In reply to | #1197171 |
On 07/31/2015 12:20 PM, Viresh Kumar wrote:
> On 31-07-15, 11:04, Murali Karicheri wrote:
>> On 07/31/2015 04:38 AM, Viresh Kumar wrote:
>>> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there
>>> is no need to do that again from its callers. Drop it.
>>>
>>
>> IS_ERR_OR_NULL() is defined as
>>
>> static inline bool __must_check IS_ERR_OR_NULL(__force const void *ptr)
>> {
>> return !ptr || IS_ERR_VALUE((unsigned long)ptr);
>> }
>>
>> So the unlikely() applies only to second part. Wouldn't that be a
>> problem for optimization?
>
> This is what the first patch of the series does:
>
> http://permalink.gmane.org/gmane.linux.kernel/2009151
>
Assuming the above change is merged, this patch looks good.
Acked-by: Murali Karicheri <m-karicheri2@ti.com>
--
Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web