Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
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 -- Murali Karicheri Linux Kernel, Keystone -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
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 netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
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 netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
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 --- 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 netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/15] drivers: net: Drop unlikely before IS_ERR(_OR_NULL)
IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there is no need to do that again from its callers. Drop it. Signed-off-by: Viresh Kumar --- 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; -- 2.4.0 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html