mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Lothar Waßmann" <LW@KARO-electronics.de>
To: David Miller <davem@davemloft.net>
Cc: fabio.estevam@freescale.com, Frank.Li@freescale.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	rmk+kernel@arm.linux.org.uk,
	linux-arm-kernel@lists.infradead.org
Subject: Re: net: fec: fix regression on i.MX28 introduced by rx_copybreak support
Date: Wed, 5 Nov 2014 06:21:48 +0100	[thread overview]
Message-ID: <20141105062148.2a997b95@ipc1.ka-ro> (raw)
In-Reply-To: <20141104.112858.356827532569349127.davem@davemloft.net>

Hi,

David Miller wrote:
> From: Lothar Waßmann <LW@KARO-electronics.de>
> Date: Tue, 4 Nov 2014 11:29:12 +0100
> 
> > Hi David,
> > 
> > Lothar Waßmann wrote:
> >> David Miller wrote:
> >> > From: Lothar Waßmann <LW@KARO-electronics.de>
> >> > Date: Thu, 30 Oct 2014 07:51:04 +0100
> >> > 
> >> > >> Also, I don't thnk your DIV_ROUND_UP() eliminate for the loop
> >> > >> in swap_buffer() is valid.  The whole point is that the current
> >> > >> code handles buffers which have a length which is not a multiple
> >> > >> of 4 properly, after your change it will no longer do so.
> >> > >>
> >> > > Do you really think so?
> >> > 
> >> > Yes, because you're rounding down so you'll miss the final
> >> > partial word (if any).
> >> >
> >> Nope. DIV_ROUND_UP() would give '1' as upper bound for lengths from 1 to
> >> 4, '2' for lengths from 5 to 8 and so on.
> >> 
> >> The loop with increment 4 and i < len does exactly the same.
> >> Try it for yourself, if you don't believe it.
> >> 
> >> 
> > Do you still think, the loop without DIV_ROUND_UP() is incorrect,
> > or can this patch be applied?
> 
> I haven't had the time to fully re-look into the details, I'm busy
> with many other things at the moment.
> 
> But looking at DIV_ROUND_UP() macro it rounds up.  It gives an
> upper bound of 4 for any value 1 to 4.  Unlike what you claim.
> 
You're missing the 'DIV' part of DIV_ROUND_UP().

> Because it goes "(n + (d - 1)) / d"
> 
> Which for 'd' of 4 gives:
> 
> 1 --> 4
> 2 --> 4
> 3 --> 4
> 4 --> 4
>
'1', not '4'.
The loop has to be done once for each (probably partial) WORD of input
data, not for each BYTE.

Without dividing and incrementing by four the loop
counter will be 0 for the first word which is less than any length > 0.
Thus the loop will be run once for any number of bytes from 1 thru 4.
If incremented by 4 after the first loop, it will be run again for any
length > 5 and so forth.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________

  reply	other threads:[~2014-11-05  5:22 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-28 11:01 [PATCHv2 0/6] " Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 1/6] net: fec: indentation cleanup; no functional change Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 2/6] net: fec: declare bufdesc_ex flag as bool Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 3/6] net: fec: improve access to quirk flags by copying them into fec_enet_private struct Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 4/6] net: fec: use swab32s() instead of cpu_to_be32() Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 5/6] net: fec: simplify loop counter handling in swap_buffer() Lothar Waßmann
2014-10-28 11:01 ` [PATCHv2 6/6] net: fec: fix regression on i.MX28 introduced by rx_copybreak support Lothar Waßmann
2014-10-28 11:12   ` David Laight
2014-10-28 12:36     ` Lothar Waßmann
2014-10-28 13:01       ` David Laight
2014-10-28 13:30         ` Lothar Waßmann
2014-10-28 11:14   ` David Laight
2014-10-28 12:10     ` Lothar Waßmann
2014-10-28 13:22 ` Lothar Waßmann
2014-10-28 13:22   ` [PATCHv3 1/9] net: fec: indentation cleanup Lothar Waßmann
2014-10-28 13:22   ` [PATCHv3 2/9] net: fec: consistently use lower case chars as hex digits Lothar Waßmann
2014-10-28 13:22   ` [PATCHv3 3/9] net: fec: properly parenthesize macro args Lothar Waßmann
2014-10-28 13:22   ` [PATCHv3 4/9] net: fec: declare bufdesc_ex flag as bool Lothar Waßmann
2014-10-28 13:23   ` [PATCHv3 5/9] net: fec: improve access to quirk flags by copying them into fec_enet_private struct Lothar Waßmann
2014-10-28 13:23   ` [PATCHv3 6/9] net: fec: use swab32s() instead of cpu_to_be32() Lothar Waßmann
2014-10-28 13:23   ` [PATCHv3 7/9] net: fec: simplify loop counter handling in swap_buffer() Lothar Waßmann
2014-10-28 13:23   ` [PATCHv3 8/9] net: fec: remove unused return value from swap_buffer() Lothar Waßmann
2014-10-28 13:23   ` [PATCHv3 9/9] net: fec: fix regression on i.MX28 introduced by rx_copybreak support Lothar Waßmann
2014-11-03 22:58     ` Stefan Wahren
2014-10-29 19:34   ` David Miller
2014-10-30  6:51     ` Lothar Waßmann
2014-10-30 16:17       ` David Miller
2014-10-31  5:32         ` Lothar Waßmann
2014-11-04 10:29           ` Lothar Waßmann
2014-11-04 16:28             ` David Miller
2014-11-05  5:21               ` Lothar Waßmann [this message]
2014-11-05 20:52                 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141105062148.2a997b95@ipc1.ka-ro \
    --to=lw@karo-electronics.de \
    --cc=Frank.Li@freescale.com \
    --cc=davem@davemloft.net \
    --cc=fabio.estevam@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®