[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] ios: track sub IOS devices
From: |
Jose E. Marchesi |
Subject: |
Re: [PATCH v2] ios: track sub IOS devices |
Date: |
Sun, 23 Apr 2023 15:11:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Mohammad.
> In this commit, we keep track of number of sub-devices for each
> IO space. If the user close the super-IOS, it'll become a zombie
> IOS: it's dead but the corresponding `struct ios' is not un-allocated
> yet. When user closes the sub-IOS, and if it's the last sub-IOS,
> it'll de-allocate the corresponding `struct ios'.
I like this idea. But I would suggest to move the sub-spaces business
completely to the IOS, at this point, instead of keeping a "sub" IOD.
> 2023-04-19 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
>
> * libpoke/ios.h (ios_inc_sub_dev): New function to track sub
> IO spaces.
> (ios_dec_sub_dev): Likewise.
> (ios_zombie_p): New function.
> * libpoke/ios.c (struct ios): Add new fields `zombie_p` and
> `num_sub_devs'.
> (ios_inc_sub_dev): New function definition.
> (ios_dec_sub_dev): Likewise.
> (ios_zombie_p): Likewise.
> (ios_open): Initialize new fields.
> (ios_close): Instead of deallocating the IOS, it becomes a zombie,
> if there's at least one sub-device that using this IOS.
> * libpoke/ios-dev-sub.c (struct ios_dev_sub): Use `ios' instead of
> ios id.
> (ios_dev_sub_open): Update to track base IOS.
> (ios_dev_sub_close): Likewise.
> (ios_dev_sub_pread): Use base IOS directly instead of searching
> using id.
> (ios_dev_sub_pwrite): Likewise.
> * testsuite/poke.pkl/open-sub-19.pk: New test.
> * testusite/poke.pkl/open-sub-20.pk: Likewise.
> * testsuite/Makefile.am (EXTRA_DIST): Update.
> ---
> ChangeLog | 25 +++++++++++++++++++++++++
> libpoke/ios-dev-sub.c | 26 ++++++++++++++------------
> libpoke/ios.c | 31 ++++++++++++++++++++++++++++++-
> libpoke/ios.h | 10 ++++++++++
> testsuite/Makefile.am | 2 ++
> testsuite/poke.pkl/open-sub-19.pk | 18 ++++++++++++++++++
> testsuite/poke.pkl/open-sub-20.pk | 20 ++++++++++++++++++++
> 7 files changed, 119 insertions(+), 13 deletions(-)
> create mode 100644 testsuite/poke.pkl/open-sub-19.pk
> create mode 100644 testsuite/poke.pkl/open-sub-20.pk
>
> diff --git a/ChangeLog b/ChangeLog
> index 1f790dc8..e9af4895 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,28 @@
> +2023-04-19 Mohammad-Reza Nabipoor <mnabipoor@gnu.org>
> +
> + * libpoke/ios.h (ios_inc_sub_dev): New function to track sub
> + IO spaces.
> + (ios_dec_sub_dev): Likewise.
> + (ios_zombie_p): New function.
> + * libpoke/ios.c (struct ios): Add new fields `zombie_p` and
> + `num_sub_devs'.
> + (ios_inc_sub_dev): New function definition.
> + (ios_dec_sub_dev): Likewise.
> + (ios_zombie_p): Likewise.
> + (ios_open): Initialize new fields.
> + (ios_close): Instead of deallocating the IOS, it becomes a zombie,
> + if there's at least one sub-device that using this IOS.
> + * libpoke/ios-dev-sub.c (struct ios_dev_sub): Use `ios' instead of
> + ios id.
> + (ios_dev_sub_open): Update to track base IOS.
> + (ios_dev_sub_close): Likewise.
> + (ios_dev_sub_pread): Use base IOS directly instead of searching
> + using id.
> + (ios_dev_sub_pwrite): Likewise.
> + * testsuite/poke.pkl/open-sub-19.pk: New test.
> + * testusite/poke.pkl/open-sub-20.pk: Likewise.
> + * testsuite/Makefile.am (EXTRA_DIST): Update.
> +
> 2023-04-17 Jose E. Marchesi <jemarch@gnu.org>
>
> * NEWS: Add entries for 3.1.
> diff --git a/libpoke/ios-dev-sub.c b/libpoke/ios-dev-sub.c
> index 64872cee..2a7f363e 100644
> --- a/libpoke/ios-dev-sub.c
> +++ b/libpoke/ios-dev-sub.c
> @@ -32,7 +32,7 @@
>
> struct ios_dev_sub
> {
> - int base_ios_id;
> + ios base_ios;
> ios_dev_off base;
> ios_dev_off size;
> char *name;
> @@ -76,6 +76,7 @@ ios_dev_sub_open (const char *handler, uint64_t flags, int
> *error, void *data)
> struct ios_dev_sub *sub = malloc (sizeof (struct ios_dev_sub));
> const char *p;
> char *end;
> + int base_ios_id;
> int explicit_flags_p = (flags != 0);
>
> if (sub == NULL)
> @@ -86,6 +87,7 @@ ios_dev_sub_open (const char *handler, uint64_t flags, int
> *error, void *data)
> }
>
> sub->name = NULL; /* To ease error management below. */
> + sub->base_ios = NULL;
>
> /* Flags: only IOS_F_READ and IOS_F_WRITE are allowed. */
> sub->flags = explicit_flags_p ? flags : IOS_F_READ | IOS_F_WRITE;
> @@ -104,7 +106,7 @@ ios_dev_sub_open (const char *handler, uint64_t flags,
> int *error, void *data)
> p = handler + 6;
>
> /* Parse the Id of the base IOS. This is an integer. */
> - sub->base_ios_id = strtol (p, &end, 0);
> + base_ios_id = strtol (p, &end, 0);
> if (*p != '\0' && *end == '/')
> /* Valid integer found. */;
> else
> @@ -146,7 +148,7 @@ ios_dev_sub_open (const char *handler, uint64_t flags,
> int *error, void *data)
> uint64_t iflags;
>
> /* The referred IOS should exist. */
> - base_ios = ios_search_by_id (sub->base_ios_id);
> + base_ios = ios_search_by_id (base_ios_id);
> if (base_ios == NULL)
> goto error;
>
> @@ -170,6 +172,8 @@ ios_dev_sub_open (const char *handler, uint64_t flags,
> int *error, void *data)
> free (sub);
> return NULL;
> }
> + sub->base_ios = base_ios;
> + ios_inc_sub_dev (base_ios);
> }
>
> if (error)
> @@ -189,6 +193,9 @@ ios_dev_sub_close (void *iod)
> {
> struct ios_dev_sub *sub = iod;
>
> + assert (sub->base_ios != NULL);
> +
> + ios_dec_sub_dev (sub->base_ios);
> free (sub->name);
> free (sub);
> return IOD_OK;
> @@ -201,18 +208,13 @@ ios_dev_sub_get_flags (void *iod)
> return sub->flags;
> }
>
> -/* XXX search_by_id to get the base IOS in pread and pwrite is slow as
> - shit. It would be good to cache the IOS at open time, but then we
> - have to make ios.c aware of subios so it will mark all sub-ios of a
> - given IOS when the later is closed. */
> -
> static int
> ios_dev_sub_pread (void *iod, void *buf, size_t count, ios_dev_off offset)
> {
> struct ios_dev_sub *sub = iod;
> - ios ios = ios_search_by_id (sub->base_ios_id);
> + ios ios = sub->base_ios;
>
> - if (ios == NULL || !(sub->flags & IOS_F_READ))
> + if (ios_zombie_p (ios) || !(sub->flags & IOS_F_READ))
> return IOD_ERROR;
>
> if (offset >= sub->size)
> @@ -228,9 +230,9 @@ ios_dev_sub_pwrite (void *iod, const void *buf, size_t
> count,
> ios_dev_off offset)
> {
> struct ios_dev_sub *sub = iod;
> - ios ios = ios_search_by_id (sub->base_ios_id);
> + ios ios = sub->base_ios;
>
> - if (ios == NULL || !(sub->flags & IOS_F_WRITE))
> + if (ios_zombie_p (ios) || !(sub->flags & IOS_F_WRITE))
> return IOD_ERROR;
>
> /* Sub-range IOS dot accept writes past the end of the IOS. */
> diff --git a/libpoke/ios.c b/libpoke/ios.c
> index 93da34c5..b5a893b3 100644
> --- a/libpoke/ios.c
> +++ b/libpoke/ios.c
> @@ -70,6 +70,8 @@
> struct ios
> {
> int id;
> + int zombie_p;
> + int num_sub_devs;
> char *handler;
> void *dev;
> struct ios_dev_if *dev_if;
> @@ -146,6 +148,8 @@ ios_open (const char *handler, uint64_t flags, int
> set_cur)
> if (!io)
> return IOS_ENOMEM;
>
> + io->zombie_p = 0;
> + io->num_sub_devs = 0;
> io->handler = NULL;
> io->next = NULL;
> io->bias = 0;
> @@ -245,7 +249,10 @@ ios_close (ios io)
> if (ios_next_id == io->id + 1)
> --ios_next_id;
>
> - free (io);
> + if (io->num_sub_devs == 0)
> + free (io);
> + else
> + io->zombie_p = 1;
>
> return IOD_ERROR_TO_IOS_ERROR (ret);
> }
> @@ -274,6 +281,13 @@ ios_set_cur (ios io)
> cur_io = io;
> }
>
> +int
> +ios_zombie_p (ios io)
> +{
> + assert (io);
> + return io->zombie_p;
> +}
> +
> ios
> ios_search (const char *handler)
> {
> @@ -1650,3 +1664,18 @@ ios_register_foreign_iod (struct ios_dev_if *iod_if)
> ios_dev_ifs[0] = iod_if;
> return IOS_OK;
> }
> +
> +void
> +ios_inc_sub_dev (ios io)
> +{
> + ++io->num_sub_devs;
> +}
> +
> +void
> +ios_dec_sub_dev (ios io)
> +{
> + assert (io->num_sub_devs != 0);
> + --io->num_sub_devs;
> + if (io->zombie_p && io->num_sub_devs == 0)
> + free (io);
> +}
> diff --git a/libpoke/ios.h b/libpoke/ios.h
> index b639822d..57bacedc 100644
> --- a/libpoke/ios.h
> +++ b/libpoke/ios.h
> @@ -238,6 +238,8 @@ ios_off ios_get_bias (ios io);
>
> void ios_set_bias (ios io, ios_off bias);
>
> +int ios_zobmie_p (ios io);
> +
> /* **************** Object read/write API **************** */
>
> /* An integer with flags is passed to the read/write operations,
> @@ -366,4 +368,12 @@ struct ios_dev_if *ios_foreign_iod (void);
> struct ios_dev_if;
> int ios_register_foreign_iod (struct ios_dev_if *iod_if);
>
> +/* **************** Sub IO space **************** */
> +
> +/* Increase the number of sub-devices for IOS. */
> +void ios_inc_sub_dev (ios io);
> +
> +/* Decrease the number of sub-devices for IOS. */
> +void ios_dec_sub_dev (ios io);
> +
> #endif /* ! IOS_H */
> diff --git a/testsuite/Makefile.am b/testsuite/Makefile.am
> index de4bb752..9a123340 100644
> --- a/testsuite/Makefile.am
> +++ b/testsuite/Makefile.am
> @@ -1951,6 +1951,8 @@ EXTRA_DIST = \
> poke.pkl/open-sub-16.pk \
> poke.pkl/open-sub-17.pk \
> poke.pkl/open-sub-18.pk \
> + poke.pkl/open-sub-19.pk \
> + poke.pkl/open-sub-20.pk \
> poke.pkl/open-sub-2.pk \
> poke.pkl/open-sub-3.pk \
> poke.pkl/open-sub-4.pk \
> diff --git a/testsuite/poke.pkl/open-sub-19.pk
> b/testsuite/poke.pkl/open-sub-19.pk
> new file mode 100644
> index 00000000..20aa2afb
> --- /dev/null
> +++ b/testsuite/poke.pkl/open-sub-19.pk
> @@ -0,0 +1,18 @@
> +/* { dg-do run } */
> +/* { dg-data {c*} {0x10 0x20 0x30 0x40 0x50 0x60 0x70 0x80 0x90 0xa0 0xb0
> 0xc0} foo } */
> +
> +/* Closing the super-IOS before the sub-IOS. */
> +
> +/* { dg-command {.set obase 16} } */
> +/* { dg-command {var file = open ("foo")} } */
> +/* { dg-command {var sub = opensub (file, 4#B, 6#B)} } */
> +/* { dg-command {byte @ sub : 1#B} } */
> +/* { dg-output "0x60UB" } */
> +/* { dg-command {byte @ file : 5#B} } */
> +/* { dg-output "\n0x60UB" } */
> +/* { dg-command {close (file)} } */
> +/* { dg-command {(byte @ sub : 0#B) ?! E_io} } */
> +/* { dg-output "\n0x00000001" } */
> +/* { dg-command {close (sub)} } */
> +/* { dg-command {(byte @ sub : 0#B) ?! E_no_ios} } */
> +/* { dg-output "\n0x00000001" } */
> diff --git a/testsuite/poke.pkl/open-sub-20.pk
> b/testsuite/poke.pkl/open-sub-20.pk
> new file mode 100644
> index 00000000..0c0f81ef
> --- /dev/null
> +++ b/testsuite/poke.pkl/open-sub-20.pk
> @@ -0,0 +1,20 @@
> +/* { dg-do run } */
> +/* { dg-data {c*} {0x10 0x20 0x30 0x40 0x50 0x60 0x70 0x80 0x90 0xa0 0xb0
> 0xc0} foo } */
> +
> +/* Closing the super-IOS before the sub-IOS. */
> +
> +/* { dg-command {.set obase 16} } */
> +/* { dg-command {var file = open ("foo")} } */
> +/* { dg-command {var sub1 = opensub (file, 4#B, 6#B, "")} } */
> +/* { dg-command {var sub2 = opensub (sub1, 1#B, 2#B, "")} } */
> +/* { dg-command {byte @ sub2 : 0#B = 0xff} } */
> +/* { dg-command {byte @ sub1 : 1#B} } */
> +/* { dg-output "0xffUB" } */
> +/* { dg-command {byte @ file : 5#B} } */
> +/* { dg-output "\n0xffUB" } */
> +/* { dg-command {close (sub1)} } */
> +/* { dg-command {(byte @ sub2 : 0#B) ?! E_io} } */
> +/* { dg-output "\n0x00000001" } */
> +/* { dg-command {close (sub2)} } */
> +/* { dg-command {(byte @ sub2 : 0#B) ?! E_no_ios} } */
> +/* { dg-output "\n0x00000001" } */