| Message ID | 20160127211216-mutt-send-email-mst@redhat.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jan 27, 2016 at 11:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > > arch/sh/include/asm/barrier.h | 1 - > tools/virtio/asm/barrier.h | 22 +- > tools/virtio/linux/compiler.h | 9 + > tools/virtio/linux/kernel.h | 1 + > tools/virtio/ringtest/main.h | 119 ++++++++++ > drivers/virtio/virtio_pci_common.c | 2 + > tools/virtio/ringtest/main.c | 366 +++++++++++++++++++++++++++++++ > tools/virtio/ringtest/ring.c | 272 +++++++++++++++++++++++ > tools/virtio/ringtest/virtio_ring_0_9.c | 316 ++++++++++++++++++++++++++ > tools/virtio/ringtest/virtio_ring_poll.c | 2 + > tools/virtio/ringtest/Makefile | 22 ++ > tools/virtio/ringtest/README | 2 + > tools/virtio/ringtest/run-on-all.sh | 24 ++ > 13 files changed, 1148 insertions(+), 10 deletions(-) How the heck did you generate that diffstat? The names should be ordered, and are for me. Anyway, pulled. Just curious about how that thing happened. Linus
On Wed, Jan 27, 2016 at 11:57:52AM -0800, Linus Torvalds wrote: > On Wed, Jan 27, 2016 at 11:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote: > > > > arch/sh/include/asm/barrier.h | 1 - > > tools/virtio/asm/barrier.h | 22 +- > > tools/virtio/linux/compiler.h | 9 + > > tools/virtio/linux/kernel.h | 1 + > > tools/virtio/ringtest/main.h | 119 ++++++++++ > > drivers/virtio/virtio_pci_common.c | 2 + > > tools/virtio/ringtest/main.c | 366 +++++++++++++++++++++++++++++++ > > tools/virtio/ringtest/ring.c | 272 +++++++++++++++++++++++ > > tools/virtio/ringtest/virtio_ring_0_9.c | 316 ++++++++++++++++++++++++++ > > tools/virtio/ringtest/virtio_ring_poll.c | 2 + > > tools/virtio/ringtest/Makefile | 22 ++ > > tools/virtio/ringtest/README | 2 + > > tools/virtio/ringtest/run-on-all.sh | 24 ++ > > 13 files changed, 1148 insertions(+), 10 deletions(-) > > How the heck did you generate that diffstat? The names should be > ordered, and are for me. > > Anyway, pulled. Just curious about how that thing happened. > > Linus That's because apparently diffstat obeys orderfile rules: [diff] renames = true orderfile = "/home/mst/.gitorderfile" $ cat .gitorderfile configure Makefile* *.json *.h *.c
On Wed, Jan 27, 2016 at 2:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote: >> >> Anyway, pulled. Just curious about how that thing happened. > > That's because apparently diffstat obeys orderfile rules: Ugh. I guess that makes sense, but it's still very annoying for something like a pull request, where now different people end up having different diffstats. And the reason I never noticed it is that likely there aren't that many people who use an orderfile. I guess something like "-O /dev/null" in the pull-request would undo it, but it is a bit annoying. I've never actually met anybody (knowingly) that used that option. I thought it was a Junio-only use case (it's been around forever as a command line option, but the config file entry seems to be somewhat recent and I wasn't even aware of it). Adding Junio just as background to see what he thinks. Looks like the diff.orderfile config option hits not just porcelain, but plumbing too. Linus
Linus Torvalds <torvalds@linux-foundation.org> writes: > On Wed, Jan 27, 2016 at 2:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote: >>> >>> Anyway, pulled. Just curious about how that thing happened. >> >> That's because apparently diffstat obeys orderfile rules: > > Ugh. I guess that makes sense, but it's still very annoying for > something like a pull request, where now different people end up > having different diffstats. And the reason I never noticed it is that > likely there aren't that many people who use an orderfile. > > I guess something like "-O /dev/null" in the pull-request would undo > it, but it is a bit annoying. > > I've never actually met anybody (knowingly) that used that option. I > thought it was a Junio-only use case (it's been around forever as a > command line option, but the config file entry seems to be somewhat > recent and I wasn't even aware of it). > > Adding Junio just as background to see what he thinks. Looks like the > diff.orderfile config option hits not just porcelain, but plumbing > too. Note: without enough context, I am guessing that you are annoyed that diffstat graph in a pull request did not match what you got locally after pulling. I've never actually met anybody who uses the orderfile, either. And I tend to agree that it probably is a mistake if the configuration is not limited to diff_ui_config(). Wait. It _is_ limited to diff_ui_config(). The thing is, "git request-pull" does use the Porcelain "git diff" to emit the final diffstat. So there is nothing to fix in diff.c, at least ;-) I am of two minds about the configuration affecting the output from "git request-pull" command as a whole. In a sense, "request-pull" command itself _is_ a UI level thing, i.e. Porcelain, and if a project chooses to standardize on a certain patch/diff presentation order using the diff.orderfile facility, there should be a way to tell your project participants to use the same order when they send in their patches and their pull requests, but with the hardcoded "git diff --stat" invocation at the end of "git request-pull", such a per-project customization must happen via _some_ configuration variable. We _could_ make up request-pull.difforderfile that is read by that script and gets turned into "-O $file" arguments to the "git diff" invocation, but if projects really wants to standardize on a single patch/diff presentation order, I do not think such a split configuration buys us anything. We'd be better off using a single diff.orderfile and have it consistently honoured by tools everybody who participates in the project uses. You obviously can declare "With MY project, the standard order in which you must present patch/diff is with this empty file", and tell your project participants to set diff.orderfile to /dev/null in the repository config when they work on the kernel. That would be what any such hypothetical project would do, when it has a specific preference of the patch/diff presentation order. After all, you do have a specific preference, which is "do not futz with the order in which Git gives the paths by default", so...
The following changes since commit 92e963f50fc74041b5e9e744c330dca48e04f08d: Linux 4.5-rc1 (2016-01-24 13:06:47 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus for you to fetch changes up to 481eaec37e91e2b33f17275901172f50ce2c71e8: tools/virtio: add ringtest utilities (2016-01-26 10:18:30 +0200) ---------------------------------------------------------------- virtio: fixes, tests This fixes existing tests broken by barrier rework, and adds some new tests. Plus, there's a fix for an old bug in virtio-pci. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> ---------------------------------------------------------------- Michael S. Tsirkin (4): virtio_pci: fix use after free on release tools/virtio: use virt_xxx barriers sh: fix smp_store_mb for !SMP tools/virtio: add ringtest utilities arch/sh/include/asm/barrier.h | 1 - tools/virtio/asm/barrier.h | 22 +- tools/virtio/linux/compiler.h | 9 + tools/virtio/linux/kernel.h | 1 + tools/virtio/ringtest/main.h | 119 ++++++++++ drivers/virtio/virtio_pci_common.c | 2 + tools/virtio/ringtest/main.c | 366 +++++++++++++++++++++++++++++++ tools/virtio/ringtest/ring.c | 272 +++++++++++++++++++++++ tools/virtio/ringtest/virtio_ring_0_9.c | 316 ++++++++++++++++++++++++++ tools/virtio/ringtest/virtio_ring_poll.c | 2 + tools/virtio/ringtest/Makefile | 22 ++ tools/virtio/ringtest/README | 2 + tools/virtio/ringtest/run-on-all.sh | 24 ++ 13 files changed, 1148 insertions(+), 10 deletions(-) create mode 100644 tools/virtio/linux/compiler.h create mode 100644 tools/virtio/ringtest/main.h create mode 100644 tools/virtio/ringtest/main.c create mode 100644 tools/virtio/ringtest/ring.c create mode 100644 tools/virtio/ringtest/virtio_ring_0_9.c create mode 100644 tools/virtio/ringtest/virtio_ring_poll.c create mode 100644 tools/virtio/ringtest/Makefile create mode 100644 tools/virtio/ringtest/README create mode 100755 tools/virtio/ringtest/run-on-all.sh