poke-devel
[Top][All Lists]
Advanced

[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" } */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]