Message ID | 20210518183655.1711377-1-philmd@redhat.com |
---|---|
Headers | show |
Series | exec: Add load/store API for aligned pointers | expand |
Patchew URL: https://patchew.org/QEMU/20210518183655.1711377-1-philmd@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210518183655.1711377-1-philmd@redhat.com Subject: [RFC PATCH 00/25] exec: Add load/store API for aligned pointers === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210518183655.1711377-1-philmd@redhat.com -> patchew/20210518183655.1711377-1-philmd@redhat.com Switched to a new branch 'test' b51497a hw/virtio: Optimize accesses on vring aligned pointers f7d21cb hw/virtio: Add methods for aligned pointer access 3ad61c3 hw/virtio: Use LDST_CONVERT macro to emit 64-bit load/store functions c03a99d hw/virtio: Use LDST_CONVERT macro to emit 32-bit load/store functions 5f7cb6e hw/virtio: Use ST_CONVERT() macro to emit 16-bit load/store functions d95d346 hw/virtio: Introduce VIRTIO_ST_CONVERT() macro fed8880 hw/virtio: Use LD_CONVERT macro to emit 16-bit unsigned load/store code cfffb55 hw/virtio: Introduce VIRTIO_LD_CONVERT() macro 0c89b29 hw/virtio: Use correct type sizes 66d8c15 exec/memory: Add methods for aligned pointer access (physical memory) 9a169b5 exec/memory: Add methods for aligned pointer access (address space) 493016c qemu/bswap: Introduce load/store for aligned pointer 5cfe6c7 qemu/bswap: Use LDST_CONVERT macro to emit 64-bit load/store functions 4256f3f qemu/bswap: Use LDST_CONVERT macro to emit 32-bit load/store functions 5e69992 qemu/bswap: Use LD_CONVERT macro to emit 16-bit unsigned load/store code 0fa8f4d qemu/bswap: Use LD_CONVERT macro to emit 16-bit signed load/store code 2a6c3d1 qemu/bswap: Introduce LD_CONVERT() macro e7b6792 qemu/bswap: Use ST_CONVERT() macro to emit 16-bit load/store functions 0d0c3e4 qemu/bswap: Introduce ST_CONVERT() macro 1ca0493 exec/memory: Use correct type size 3010882 exec/memory_ldst_cached: Use correct type size 0beaa26 exec/memory_ldst_phys: Use correct type sizes 88d6831 exec/memory_ldst: Use correct type sizes 057db27 exec/memory_ldst_phys: Sort declarations 3a92956 exec/memory_ldst_cached: Sort declarations === OUTPUT BEGIN === 1/25 Checking commit 3a9295699cb2 (exec/memory_ldst_cached: Sort declarations) 2/25 Checking commit 057db27b4129 (exec/memory_ldst_phys: Sort declarations) 3/25 Checking commit 88d6831d2688 (exec/memory_ldst: Use correct type sizes) 4/25 Checking commit 0beaa26979fa (exec/memory_ldst_phys: Use correct type sizes) 5/25 Checking commit 3010882b5604 (exec/memory_ldst_cached: Use correct type size) 6/25 Checking commit 1ca0493ecfd1 (exec/memory: Use correct type size) 7/25 Checking commit 0d0c3e4e5c2d (qemu/bswap: Introduce ST_CONVERT() macro) 8/25 Checking commit e7b679286247 (qemu/bswap: Use ST_CONVERT() macro to emit 16-bit load/store functions) 9/25 Checking commit 2a6c3d1c4aad (qemu/bswap: Introduce LD_CONVERT() macro) 10/25 Checking commit 0fa8f4d7d229 (qemu/bswap: Use LD_CONVERT macro to emit 16-bit signed load/store code) 11/25 Checking commit 5e69992e047b (qemu/bswap: Use LD_CONVERT macro to emit 16-bit unsigned load/store code) 12/25 Checking commit 4256f3f7f36a (qemu/bswap: Use LDST_CONVERT macro to emit 32-bit load/store functions) 13/25 Checking commit 5cfe6c7a8fd6 (qemu/bswap: Use LDST_CONVERT macro to emit 64-bit load/store functions) 14/25 Checking commit 493016c0c84e (qemu/bswap: Introduce load/store for aligned pointer) ERROR: space required after that close brace '}' #119: FILE: include/qemu/bswap.h:369: +}\ ERROR: space required after that close brace '}' #129: FILE: include/qemu/bswap.h:379: +}\ total: 2 errors, 0 warnings, 107 lines checked Patch 14/25 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 15/25 Checking commit 9a169b58ddf3 (exec/memory: Add methods for aligned pointer access (address space)) 16/25 Checking commit 66d8c1562136 (exec/memory: Add methods for aligned pointer access (physical memory)) 17/25 Checking commit 0c89b296d850 (hw/virtio: Use correct type sizes) 18/25 Checking commit cfffb555f1ca (hw/virtio: Introduce VIRTIO_LD_CONVERT() macro) ERROR: space required after that close brace '}' #31: FILE: include/hw/virtio/virtio-access.h:49: + }\ ERROR: space required after that close brace '}' #33: FILE: include/hw/virtio/virtio-access.h:51: +}\ ERROR: space required after that close brace '}' #41: FILE: include/hw/virtio/virtio-access.h:59: + }\ ERROR: space required after that close brace '}' #42: FILE: include/hw/virtio/virtio-access.h:60: +}\ ERROR: space required after that close brace '}' #49: FILE: include/hw/virtio/virtio-access.h:67: + }\ total: 5 errors, 0 warnings, 35 lines checked Patch 18/25 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 19/25 Checking commit fed888096371 (hw/virtio: Use LD_CONVERT macro to emit 16-bit unsigned load/store code) 20/25 Checking commit d95d346351f6 (hw/virtio: Introduce VIRTIO_ST_CONVERT() macro) ERROR: space required after that close brace '}' #32: FILE: include/hw/virtio/virtio-access.h:79: + }\ ERROR: space required after that close brace '}' #33: FILE: include/hw/virtio/virtio-access.h:80: +}\ ERROR: space required after that close brace '}' #43: FILE: include/hw/virtio/virtio-access.h:90: + }\ ERROR: space required after that close brace '}' #44: FILE: include/hw/virtio/virtio-access.h:91: +}\ ERROR: space required after that close brace '}' #53: FILE: include/hw/virtio/virtio-access.h:100: + }\ total: 5 errors, 0 warnings, 38 lines checked Patch 20/25 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 21/25 Checking commit 5f7cb6e5bf21 (hw/virtio: Use ST_CONVERT() macro to emit 16-bit load/store functions) 22/25 Checking commit c03a99d5100a (hw/virtio: Use LDST_CONVERT macro to emit 32-bit load/store functions) 23/25 Checking commit 3ad61c32db83 (hw/virtio: Use LDST_CONVERT macro to emit 64-bit load/store functions) 24/25 Checking commit f7d21cb701d1 (hw/virtio: Add methods for aligned pointer access) ERROR: space required after that close brace '}' #22: FILE: include/hw/virtio/virtio-access.h:69: +}\ ERROR: space required after that close brace '}' #30: FILE: include/hw/virtio/virtio-access.h:77: + }\ ERROR: space required after that close brace '}' #39: FILE: include/hw/virtio/virtio-access.h:111: +}\ ERROR: space required after that close brace '}' #49: FILE: include/hw/virtio/virtio-access.h:121: + }\ total: 4 errors, 0 warnings, 33 lines checked Patch 24/25 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 25/25 Checking commit b51497a1e1f3 (hw/virtio: Optimize accesses on vring aligned pointers) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210518183655.1711377-1-philmd@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 5/18/21 1:36 PM, Philippe Mathieu-Daudé wrote: > The series is decomposed as: > - cleanups (1-6) > - clean ldst API using macros (7-13) > - add aligned ldst methods (14) > - add aligned memory methods (15-16) > - similar changes in virtio (17-24) > - use the new methods on vring aligned values (25) > > There are some checkpatch errors related to the macro used. I think we should emphasize the atomicness of the access as opposed to the alignedness. That's the only thing that's important to virtio. Thus s/aligned/atomic/g and use qatomic_read/qatomic_set. r~
On 5/19/21 9:20 PM, Richard Henderson wrote: > On 5/18/21 1:36 PM, Philippe Mathieu-Daudé wrote: >> The series is decomposed as: >> - cleanups (1-6) >> - clean ldst API using macros (7-13) >> - add aligned ldst methods (14) >> - add aligned memory methods (15-16) >> - similar changes in virtio (17-24) >> - use the new methods on vring aligned values (25) >> >> There are some checkpatch errors related to the macro used. > > I think we should emphasize the atomicness of the access as opposed to > the alignedness. That's the only thing that's important to virtio. > > Thus s/aligned/atomic/g and use qatomic_read/qatomic_set. OK. Do you think patches 1-6, 17 (Use correct type sizes) could go in meanwhile, or should I repost them separately?
On 5/19/21 2:40 PM, Philippe Mathieu-Daudé wrote: > On 5/19/21 9:20 PM, Richard Henderson wrote: >> On 5/18/21 1:36 PM, Philippe Mathieu-Daudé wrote: >>> The series is decomposed as: >>> - cleanups (1-6) >>> - clean ldst API using macros (7-13) >>> - add aligned ldst methods (14) >>> - add aligned memory methods (15-16) >>> - similar changes in virtio (17-24) >>> - use the new methods on vring aligned values (25) >>> >>> There are some checkpatch errors related to the macro used. >> >> I think we should emphasize the atomicness of the access as opposed to >> the alignedness. That's the only thing that's important to virtio. >> >> Thus s/aligned/atomic/g and use qatomic_read/qatomic_set. > > OK. > > Do you think patches 1-6, 17 (Use correct type sizes) could > go in meanwhile, or should I repost them separately? > I think 1-6 can be queued; I'm had a question about 17. r~