Re: [PULL] virtio, vhost: new device, fixes, speedups

Re: [PULL] virtio, vhost: new device, fixes, speedups

[Linus Torvalds]

On Thu, Dec 15, 2016 at 3:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

Pulled, but I wonder...

>  Documentation/translations/zh_CN/sparse.txt        |   7 +-
>  arch/arm/plat-samsung/include/plat/gpio-cfg.h      |   2 +-
>  drivers/crypto/virtio/virtio_crypto_common.h       | 128 +++++
[...]

what are you generating these diffstats with? Because they are pretty bogus..

The end result is correct:

>  86 files changed, 2106 insertions(+), 280 deletions(-)

but the file order in the diffstat is completely random, which makes
it very hard to compare with what I get. It also makes it hard to see
what you changed, because it's not alphabetical like it should be
(strictly speaking the git pathname ordering isnt' really
alphabetical, since the '/' sorts as the NUL character, but close
enough).

I can't see the logic to the re-ordering of the lines, so I'm
intrigued how you even generated it.

            Linus

Re: [PULL] virtio, vhost: new device, fixes, speedups

[Paolo Bonzini]

On 16/12/2016 03:20, Linus Torvalds wrote:
> On Thu, Dec 15, 2016 at 3:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> 
> Pulled, but I wonder...
> 
>>  Documentation/translations/zh_CN/sparse.txt        |   7 +-
>>  arch/arm/plat-samsung/include/plat/gpio-cfg.h      |   2 +-
>>  drivers/crypto/virtio/virtio_crypto_common.h       | 128 +++++
> [...]
> 
> what are you generating these diffstats with? Because they are pretty bogus..
> 
> The end result is correct:
> 
>>  86 files changed, 2106 insertions(+), 280 deletions(-)
> 
> but the file order in the diffstat is completely random, which makes
> it very hard to compare with what I get. It also makes it hard to see
> what you changed, because it's not alphabetical like it should be
> (strictly speaking the git pathname ordering isnt' really
> alphabetical, since the '/' sorts as the NUL character, but close
> enough).
> 
> I can't see the logic to the re-ordering of the lines, so I'm
> intrigued how you even generated it.

Looks like a diff.orderFile that places .h and .txt first, then .c, then
Makefile.  I've seen others propose it.

Paolo

Re: [PULL] virtio, vhost: new device, fixes, speedups

[Michael S. Tsirkin]

On Thu, Dec 15, 2016 at 06:20:40PM -0800, Linus Torvalds wrote:
> On Thu, Dec 15, 2016 at 3:05 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> 
> Pulled, but I wonder...
> 
> >  Documentation/translations/zh_CN/sparse.txt        |   7 +-
> >  arch/arm/plat-samsung/include/plat/gpio-cfg.h      |   2 +-
> >  drivers/crypto/virtio/virtio_crypto_common.h       | 128 +++++
> [...]
> 
> what are you generating these diffstats with? Because they are pretty bogus..
> 
> The end result is correct:
> 
> >  86 files changed, 2106 insertions(+), 280 deletions(-)
> 
> but the file order in the diffstat is completely random, which makes
> it very hard to compare with what I get. It also makes it hard to see
> what you changed, because it's not alphabetical like it should be
> (strictly speaking the git pathname ordering isnt' really
> alphabetical, since the '/' sorts as the NUL character, but close
> enough).
> 
> I can't see the logic to the re-ordering of the lines, so I'm
> intrigued how you even generated it.
> 
>             Linus


Oh, that's because I set orderfile globally rather than
just for the qemu project which wants it.
Fixed, sorry about that.

-- 
MST

Re: [PULL] virtio, vhost: new device, fixes, speedups

[Linus Torvalds]

On Fri, Dec 16, 2016 at 9:09 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> Oh, that's because I set orderfile globally rather than
> just for the qemu project which wants it.
> Fixed, sorry about that.

That explains it. I should have remembered, I think this came up once
before with somebody else.

Yeah, for the kernel it makes things much easier (at least for me) to
have everything just the default alphabetical ordering, particularly
because we use directory structure for maintenance areas.

So ordering the diffs by type ends up breaking my mental model for "is
this pull request touching the right files", which is why I reacted.

             Linus