avg-I commented on this pull request.
> @@ -442,18 +444,26 @@ zio_remove_child(zio_t *pio, zio_t *cio, zio_link_t *zl)
}
static boolean_t
-zio_wait_for_children(zio_t *zio, enum zio_child child, enum zio_wait_type
wait)
+zio_wait_for_children(zio_t *zio, enum zio_child children,
Some compilers will complain about `children` parameter, because the values
passed through it are not of `enum zio_child` type. For example,
`ZIO_CHILD_VDEV | ZIO_CHILD_GANG` equals three which is not a valid value for
that enumeration.
> @@ -209,12 +209,13 @@ enum zio_flag {
ZIO_FLAG_CANFAIL)
enum zio_child {
- ZIO_CHILD_VDEV = 0,
- ZIO_CHILD_GANG,
- ZIO_CHILD_DDT,
- ZIO_CHILD_LOGICAL,
- ZIO_CHILD_TYPES
+ ZIO_CHILD_VDEV = 1 << 0,
+ ZIO_CHILD_GANG = 1 << 1,
+ ZIO_CHILD_DDT = 1 << 2,
+ ZIO_CHILD_LOGICAL = 1 << 3
I do not have any argument against this change, but have you considered keeping
this enumeration as is, but instead changing only `zio_wait_for_children` to
expect the bit-mask?
E.g. something like
```
#define ZIO_CHILD_BIT(c) (1 << (c))
```
and then
```
if (zio_wait_for_children(zio,
ZIO_CHILD_BIT(ZIO_CHILD_GANG) | ZIO_CHILD_BIT(ZIO_CHILD_LOGICAL),
ZIO_WAIT_READY)) {
...
}
```
Looks a bit too verbose, but has a benefit of affecting only
`zio_wait_for_children` calls which need to be modified in either case.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/505#pullrequestreview-84737942
------------------------------------------
openzfs-developer
Archives:
https://openzfs.topicbox.com/groups/developer/discussions/Td0adf1bfacf39ae0-Mb1599e3a1263637729e89df2
Powered by Topicbox: https://topicbox.com
