Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-15 Thread Greg Kurz
On Mon, 15 Mar 2021 17:07:50 +0100
Christian Schoenebeck  wrote:

> On Samstag, 13. März 2021 08:51:21 CET Greg Kurz wrote:
> > On Sat, 13 Mar 2021 07:43:38 +0200
> > 
> > Mahmoud Mandour  wrote:
> > > Thanks for the fast review. I asked on the QEMU IRC channel
> > > before committing whether to put all the changes into one patch
> > > or split them and was instructed that it was better to split them up.
> > > But in any case I was open to both ways and you can decide
> > > on the best way to go.
> > 
> > People only do inline replies here. Please don't top-post for the
> > sake of clarity.
> > 
> > So, the instructions to split the patches is obviously the way to go. The
> > question here is rather : will each subsystem maintainer pick up patches
> > from this series or will only one maintainer pick up all the patches after
> > they have been acked by the other maintainers ?
> 
> We need a call here. :)
> 
> Soft freeze is tomorrow, so will one submaintainer handle this series all 
> together or should each submaintainer handle only their specific patches?
> 

I see that some of Mahmoud's patches in Dave Gilbert's latest PR, so I
guess you can go ahead and merge this one in the 9p tree.

> > > On Thu, Mar 11, 2021 at 1:59 PM Christian Schoenebeck <
> > > 
> > > qemu_...@crudebyte.com> wrote:
> > > > On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> > > > > On Thu, 11 Mar 2021 11:49:06 +0100
> > > > > 
> > > > > Christian Schoenebeck  wrote:
> > > > > > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > > > > > Replaced a call to qemu_mutex_lock and its respective call to
> > > > > > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > > > > > This simplifies the code by removing the call required to unlock
> > > > > > > and also eliminates goto paths.
> > > > > > > 
> > > > > > > Signed-off-by: Mahmoud Mandour 
> > > > > > > ---
> > > > > > > 
> > > > > > >  hw/9pfs/9p-synth.c | 12 
> > > > > > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > > > > > index 7eb210ffa8..473ef914b0 100644
> > > > > > > --- a/hw/9pfs/9p-synth.c
> > > > > > > +++ b/hw/9pfs/9p-synth.c
> > > > > > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode
> > > > > > > *parent,
> > > > 
> > > > int
> > > > 
> > > > > > > mode, if (!parent) {
> > > > > > > 
> > > > > > >  parent = _root;
> > > > > > >  
> > > > > > >  }
> > > > > > > 
> > > > > > > -qemu_mutex_lock(_mutex);
> > > > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > > > > 
> > > > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > > > >  
> > > > > > >  if (!strcmp(tmp->name, name)) {
> > > > > > >  
> > > > > > >  ret = EEXIST;
> > > > > > > 
> > > > > > > -goto err_out;
> > > > > > > +return ret;
> > > > > > > 
> > > > > > >  }
> > > > > > >  
> > > > > > >  }
> > > > > > >  /* Add the name */
> > > > > > > 
> > > > > > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> > > > 
> > > > int
> > > > 
> > > > > > > mode, node->attr, node->attr->inode);
> > > > > > > 
> > > > > > >  *result = node;
> > > > > > >  ret = 0;
> > > > > > > 
> > > > > > > -err_out:
> > > > > > > -qemu_mutex_unlock(_mutex);
> > > > > > > 
> > > > > > >  return ret;
> > > > > > >  
> > > > > > >  }
> > > > > > > 
> > > > > > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > > > > > *parent,
> > > > > > > int mode, parent = _root;
> > > > > > > 
> > > > > > >  }
> > > > > > > 
> > > > > > > -qemu_mutex_lock(_mutex);
> > > > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > > > > 
> > > > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > > > >  
> > > > > > >  if (!strcmp(tmp->name, name)) {
> > > > > > >  
> > > > > > >  ret = EEXIST;
> > > > > > > 
> > > > > > > -goto err_out;
> > > > > > > +return ret;
> > > > > > > 
> > > > > > >  }
> > > > > > >  
> > > > > > >  }
> > > > > > >  /* Add file type and remove write bits */
> > > > > > > 
> > > > > > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > > 
> > > > *parent,
> > > > 
> > > > > > > int
> > > > > > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > > > > > > 
> > > > > > >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> > > > > > >  ret = 0;
> > > > > > > 
> > > > > > > -err_out:
> > > > > > > -qemu_mutex_unlock(_mutex);
> > > > > > > 
> > > > > > >  return ret;
> > > > > > >  
> > > > > > >  }
> > > > > > 
> > > > > > Reviewed-by: Christian Schoenebeck 
> > > > > > 
> > > > > > Greg, I suggest I'll push this onto my queue as you seem to be busy.
> > > > > 
> > > > > This cleanup spans over multiple subsystems but I think it makes more
> > > > > sense to keep all these patches together. Let's wait 

Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-15 Thread Christian Schoenebeck
On Samstag, 13. März 2021 08:51:21 CET Greg Kurz wrote:
> On Sat, 13 Mar 2021 07:43:38 +0200
> 
> Mahmoud Mandour  wrote:
> > Thanks for the fast review. I asked on the QEMU IRC channel
> > before committing whether to put all the changes into one patch
> > or split them and was instructed that it was better to split them up.
> > But in any case I was open to both ways and you can decide
> > on the best way to go.
> 
> People only do inline replies here. Please don't top-post for the
> sake of clarity.
> 
> So, the instructions to split the patches is obviously the way to go. The
> question here is rather : will each subsystem maintainer pick up patches
> from this series or will only one maintainer pick up all the patches after
> they have been acked by the other maintainers ?

We need a call here. :)

Soft freeze is tomorrow, so will one submaintainer handle this series all 
together or should each submaintainer handle only their specific patches?

> > On Thu, Mar 11, 2021 at 1:59 PM Christian Schoenebeck <
> > 
> > qemu_...@crudebyte.com> wrote:
> > > On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> > > > On Thu, 11 Mar 2021 11:49:06 +0100
> > > > 
> > > > Christian Schoenebeck  wrote:
> > > > > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > > > > Replaced a call to qemu_mutex_lock and its respective call to
> > > > > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > > > > This simplifies the code by removing the call required to unlock
> > > > > > and also eliminates goto paths.
> > > > > > 
> > > > > > Signed-off-by: Mahmoud Mandour 
> > > > > > ---
> > > > > > 
> > > > > >  hw/9pfs/9p-synth.c | 12 
> > > > > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > > > > index 7eb210ffa8..473ef914b0 100644
> > > > > > --- a/hw/9pfs/9p-synth.c
> > > > > > +++ b/hw/9pfs/9p-synth.c
> > > > > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode
> > > > > > *parent,
> > > 
> > > int
> > > 
> > > > > > mode, if (!parent) {
> > > > > > 
> > > > > >  parent = _root;
> > > > > >  
> > > > > >  }
> > > > > > 
> > > > > > -qemu_mutex_lock(_mutex);
> > > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > > > 
> > > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > > >  
> > > > > >  if (!strcmp(tmp->name, name)) {
> > > > > >  
> > > > > >  ret = EEXIST;
> > > > > > 
> > > > > > -goto err_out;
> > > > > > +return ret;
> > > > > > 
> > > > > >  }
> > > > > >  
> > > > > >  }
> > > > > >  /* Add the name */
> > > > > > 
> > > > > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> > > 
> > > int
> > > 
> > > > > > mode, node->attr, node->attr->inode);
> > > > > > 
> > > > > >  *result = node;
> > > > > >  ret = 0;
> > > > > > 
> > > > > > -err_out:
> > > > > > -qemu_mutex_unlock(_mutex);
> > > > > > 
> > > > > >  return ret;
> > > > > >  
> > > > > >  }
> > > > > > 
> > > > > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > > > > *parent,
> > > > > > int mode, parent = _root;
> > > > > > 
> > > > > >  }
> > > > > > 
> > > > > > -qemu_mutex_lock(_mutex);
> > > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > > > 
> > > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > > >  
> > > > > >  if (!strcmp(tmp->name, name)) {
> > > > > >  
> > > > > >  ret = EEXIST;
> > > > > > 
> > > > > > -goto err_out;
> > > > > > +return ret;
> > > > > > 
> > > > > >  }
> > > > > >  
> > > > > >  }
> > > > > >  /* Add file type and remove write bits */
> > > > > > 
> > > > > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > 
> > > *parent,
> > > 
> > > > > > int
> > > > > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > > > > > 
> > > > > >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> > > > > >  ret = 0;
> > > > > > 
> > > > > > -err_out:
> > > > > > -qemu_mutex_unlock(_mutex);
> > > > > > 
> > > > > >  return ret;
> > > > > >  
> > > > > >  }
> > > > > 
> > > > > Reviewed-by: Christian Schoenebeck 
> > > > > 
> > > > > Greg, I suggest I'll push this onto my queue as you seem to be busy.
> > > > 
> > > > This cleanup spans over multiple subsystems but I think it makes more
> > > > sense to keep all these patches together. Let's wait for everyone to
> > > > ack/review and then we'll decide how to merge the patches.
> > > 
> > > Sure, makes sense.






Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-12 Thread Greg Kurz
On Sat, 13 Mar 2021 07:43:38 +0200
Mahmoud Mandour  wrote:

> Thanks for the fast review. I asked on the QEMU IRC channel
> before committing whether to put all the changes into one patch
> or split them and was instructed that it was better to split them up.
> But in any case I was open to both ways and you can decide
> on the best way to go.
> 

People only do inline replies here. Please don't top-post for the
sake of clarity.

So, the instructions to split the patches is obviously the way to go. The
question here is rather : will each subsystem maintainer pick up patches
from this series or will only one maintainer pick up all the patches after
they have been acked by the other maintainers ?

> On Thu, Mar 11, 2021 at 1:59 PM Christian Schoenebeck <
> qemu_...@crudebyte.com> wrote:
> 
> > On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> > > On Thu, 11 Mar 2021 11:49:06 +0100
> > >
> > > Christian Schoenebeck  wrote:
> > > > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > > > Replaced a call to qemu_mutex_lock and its respective call to
> > > > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > > > This simplifies the code by removing the call required to unlock
> > > > > and also eliminates goto paths.
> > > > >
> > > > > Signed-off-by: Mahmoud Mandour 
> > > > > ---
> > > > >
> > > > >  hw/9pfs/9p-synth.c | 12 
> > > > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > > > index 7eb210ffa8..473ef914b0 100644
> > > > > --- a/hw/9pfs/9p-synth.c
> > > > > +++ b/hw/9pfs/9p-synth.c
> > > > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> > int
> > > > > mode, if (!parent) {
> > > > >
> > > > >  parent = _root;
> > > > >
> > > > >  }
> > > > >
> > > > > -qemu_mutex_lock(_mutex);
> > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > >
> > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > >
> > > > >  if (!strcmp(tmp->name, name)) {
> > > > >
> > > > >  ret = EEXIST;
> > > > >
> > > > > -goto err_out;
> > > > > +return ret;
> > > > >
> > > > >  }
> > > > >
> > > > >  }
> > > > >  /* Add the name */
> > > > >
> > > > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> > int
> > > > > mode, node->attr, node->attr->inode);
> > > > >
> > > > >  *result = node;
> > > > >  ret = 0;
> > > > >
> > > > > -err_out:
> > > > > -qemu_mutex_unlock(_mutex);
> > > > >
> > > > >  return ret;
> > > > >
> > > > >  }
> > > > >
> > > > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > > > *parent,
> > > > > int mode, parent = _root;
> > > > >
> > > > >  }
> > > > >
> > > > > -qemu_mutex_lock(_mutex);
> > > > > +QEMU_LOCK_GUARD(_mutex);
> > > > >
> > > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > > >
> > > > >  if (!strcmp(tmp->name, name)) {
> > > > >
> > > > >  ret = EEXIST;
> > > > >
> > > > > -goto err_out;
> > > > > +return ret;
> > > > >
> > > > >  }
> > > > >
> > > > >  }
> > > > >  /* Add file type and remove write bits */
> > > > >
> > > > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > *parent,
> > > > > int
> > > > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > > > >
> > > > >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> > > > >  ret = 0;
> > > > >
> > > > > -err_out:
> > > > > -qemu_mutex_unlock(_mutex);
> > > > >
> > > > >  return ret;
> > > > >
> > > > >  }
> > > >
> > > > Reviewed-by: Christian Schoenebeck 
> > > >
> > > > Greg, I suggest I'll push this onto my queue as you seem to be busy.
> > >
> > > This cleanup spans over multiple subsystems but I think it makes more
> > > sense to keep all these patches together. Let's wait for everyone to
> > > ack/review and then we'll decide how to merge the patches.
> >
> > Sure, makes sense.
> >
> >
> >
> >




Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-12 Thread Mahmoud Mandour
Thanks for the fast review. I asked on the QEMU IRC channel
before committing whether to put all the changes into one patch
or split them and was instructed that it was better to split them up.
But in any case I was open to both ways and you can decide
on the best way to go.

On Thu, Mar 11, 2021 at 1:59 PM Christian Schoenebeck <
qemu_...@crudebyte.com> wrote:

> On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> > On Thu, 11 Mar 2021 11:49:06 +0100
> >
> > Christian Schoenebeck  wrote:
> > > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > > Replaced a call to qemu_mutex_lock and its respective call to
> > > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > > This simplifies the code by removing the call required to unlock
> > > > and also eliminates goto paths.
> > > >
> > > > Signed-off-by: Mahmoud Mandour 
> > > > ---
> > > >
> > > >  hw/9pfs/9p-synth.c | 12 
> > > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > > index 7eb210ffa8..473ef914b0 100644
> > > > --- a/hw/9pfs/9p-synth.c
> > > > +++ b/hw/9pfs/9p-synth.c
> > > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> int
> > > > mode, if (!parent) {
> > > >
> > > >  parent = _root;
> > > >
> > > >  }
> > > >
> > > > -qemu_mutex_lock(_mutex);
> > > > +QEMU_LOCK_GUARD(_mutex);
> > > >
> > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > >
> > > >  if (!strcmp(tmp->name, name)) {
> > > >
> > > >  ret = EEXIST;
> > > >
> > > > -goto err_out;
> > > > +return ret;
> > > >
> > > >  }
> > > >
> > > >  }
> > > >  /* Add the name */
> > > >
> > > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent,
> int
> > > > mode, node->attr, node->attr->inode);
> > > >
> > > >  *result = node;
> > > >  ret = 0;
> > > >
> > > > -err_out:
> > > > -qemu_mutex_unlock(_mutex);
> > > >
> > > >  return ret;
> > > >
> > > >  }
> > > >
> > > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > > *parent,
> > > > int mode, parent = _root;
> > > >
> > > >  }
> > > >
> > > > -qemu_mutex_lock(_mutex);
> > > > +QEMU_LOCK_GUARD(_mutex);
> > > >
> > > >  QLIST_FOREACH(tmp, >child, sibling) {
> > > >
> > > >  if (!strcmp(tmp->name, name)) {
> > > >
> > > >  ret = EEXIST;
> > > >
> > > > -goto err_out;
> > > > +return ret;
> > > >
> > > >  }
> > > >
> > > >  }
> > > >  /* Add file type and remove write bits */
> > > >
> > > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> *parent,
> > > > int
> > > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > > >
> > > >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> > > >  ret = 0;
> > > >
> > > > -err_out:
> > > > -qemu_mutex_unlock(_mutex);
> > > >
> > > >  return ret;
> > > >
> > > >  }
> > >
> > > Reviewed-by: Christian Schoenebeck 
> > >
> > > Greg, I suggest I'll push this onto my queue as you seem to be busy.
> >
> > This cleanup spans over multiple subsystems but I think it makes more
> > sense to keep all these patches together. Let's wait for everyone to
> > ack/review and then we'll decide how to merge the patches.
>
> Sure, makes sense.
>
>
>
>


Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-11 Thread Christian Schoenebeck
On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> On Thu, 11 Mar 2021 11:49:06 +0100
> 
> Christian Schoenebeck  wrote:
> > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > Replaced a call to qemu_mutex_lock and its respective call to
> > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > This simplifies the code by removing the call required to unlock
> > > and also eliminates goto paths.
> > > 
> > > Signed-off-by: Mahmoud Mandour 
> > > ---
> > > 
> > >  hw/9pfs/9p-synth.c | 12 
> > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > index 7eb210ffa8..473ef914b0 100644
> > > --- a/hw/9pfs/9p-synth.c
> > > +++ b/hw/9pfs/9p-synth.c
> > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> > > mode, if (!parent) {
> > > 
> > >  parent = _root;
> > >  
> > >  }
> > > 
> > > -qemu_mutex_lock(_mutex);
> > > +QEMU_LOCK_GUARD(_mutex);
> > > 
> > >  QLIST_FOREACH(tmp, >child, sibling) {
> > >  
> > >  if (!strcmp(tmp->name, name)) {
> > >  
> > >  ret = EEXIST;
> > > 
> > > -goto err_out;
> > > +return ret;
> > > 
> > >  }
> > >  
> > >  }
> > >  /* Add the name */
> > > 
> > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> > > mode, node->attr, node->attr->inode);
> > > 
> > >  *result = node;
> > >  ret = 0;
> > > 
> > > -err_out:
> > > -qemu_mutex_unlock(_mutex);
> > > 
> > >  return ret;
> > >  
> > >  }
> > > 
> > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > *parent,
> > > int mode, parent = _root;
> > > 
> > >  }
> > > 
> > > -qemu_mutex_lock(_mutex);
> > > +QEMU_LOCK_GUARD(_mutex);
> > > 
> > >  QLIST_FOREACH(tmp, >child, sibling) {
> > >  
> > >  if (!strcmp(tmp->name, name)) {
> > >  
> > >  ret = EEXIST;
> > > 
> > > -goto err_out;
> > > +return ret;
> > > 
> > >  }
> > >  
> > >  }
> > >  /* Add file type and remove write bits */
> > > 
> > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent,
> > > int
> > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > > 
> > >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> > >  ret = 0;
> > > 
> > > -err_out:
> > > -qemu_mutex_unlock(_mutex);
> > > 
> > >  return ret;
> > >  
> > >  }
> > 
> > Reviewed-by: Christian Schoenebeck 
> > 
> > Greg, I suggest I'll push this onto my queue as you seem to be busy.
> 
> This cleanup spans over multiple subsystems but I think it makes more
> sense to keep all these patches together. Let's wait for everyone to
> ack/review and then we'll decide how to merge the patches.

Sure, makes sense.






Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-11 Thread Greg Kurz
On Thu, 11 Mar 2021 11:49:06 +0100
Christian Schoenebeck  wrote:

> On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > Replaced a call to qemu_mutex_lock and its respective call to
> > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > This simplifies the code by removing the call required to unlock
> > and also eliminates goto paths.
> > 
> > Signed-off-by: Mahmoud Mandour 
> > ---
> >  hw/9pfs/9p-synth.c | 12 
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > index 7eb210ffa8..473ef914b0 100644
> > --- a/hw/9pfs/9p-synth.c
> > +++ b/hw/9pfs/9p-synth.c
> > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> > mode, if (!parent) {
> >  parent = _root;
> >  }
> > -qemu_mutex_lock(_mutex);
> > +QEMU_LOCK_GUARD(_mutex);
> >  QLIST_FOREACH(tmp, >child, sibling) {
> >  if (!strcmp(tmp->name, name)) {
> >  ret = EEXIST;
> > -goto err_out;
> > +return ret;
> >  }
> >  }
> >  /* Add the name */
> > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
> > node->attr, node->attr->inode);
> >  *result = node;
> >  ret = 0;
> > -err_out:
> > -qemu_mutex_unlock(_mutex);
> >  return ret;
> >  }
> > 
> > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent,
> > int mode, parent = _root;
> >  }
> > 
> > -qemu_mutex_lock(_mutex);
> > +QEMU_LOCK_GUARD(_mutex);
> >  QLIST_FOREACH(tmp, >child, sibling) {
> >  if (!strcmp(tmp->name, name)) {
> >  ret = EEXIST;
> > -goto err_out;
> > +return ret;
> >  }
> >  }
> >  /* Add file type and remove write bits */
> > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int
> > mode, pstrcpy(node->name, sizeof(node->name), name);
> >  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
> >  ret = 0;
> > -err_out:
> > -qemu_mutex_unlock(_mutex);
> >  return ret;
> >  }
> 
> Reviewed-by: Christian Schoenebeck 
> 
> Greg, I suggest I'll push this onto my queue as you seem to be busy.
> 

This cleanup spans over multiple subsystems but I think it makes more
sense to keep all these patches together. Let's wait for everyone to
ack/review and then we'll decide how to merge the patches.

> Best regards,
> Christian Schoenebeck
> 
> 




Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-11 Thread Christian Schoenebeck
On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> Replaced a call to qemu_mutex_lock and its respective call to
> qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> This simplifies the code by removing the call required to unlock
> and also eliminates goto paths.
> 
> Signed-off-by: Mahmoud Mandour 
> ---
>  hw/9pfs/9p-synth.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index 7eb210ffa8..473ef914b0 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> mode, if (!parent) {
>  parent = _root;
>  }
> -qemu_mutex_lock(_mutex);
> +QEMU_LOCK_GUARD(_mutex);
>  QLIST_FOREACH(tmp, >child, sibling) {
>  if (!strcmp(tmp->name, name)) {
>  ret = EEXIST;
> -goto err_out;
> +return ret;
>  }
>  }
>  /* Add the name */
> @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
> node->attr, node->attr->inode);
>  *result = node;
>  ret = 0;
> -err_out:
> -qemu_mutex_unlock(_mutex);
>  return ret;
>  }
> 
> @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent,
> int mode, parent = _root;
>  }
> 
> -qemu_mutex_lock(_mutex);
> +QEMU_LOCK_GUARD(_mutex);
>  QLIST_FOREACH(tmp, >child, sibling) {
>  if (!strcmp(tmp->name, name)) {
>  ret = EEXIST;
> -goto err_out;
> +return ret;
>  }
>  }
>  /* Add file type and remove write bits */
> @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int
> mode, pstrcpy(node->name, sizeof(node->name), name);
>  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
>  ret = 0;
> -err_out:
> -qemu_mutex_unlock(_mutex);
>  return ret;
>  }

Reviewed-by: Christian Schoenebeck 

Greg, I suggest I'll push this onto my queue as you seem to be busy.

Best regards,
Christian Schoenebeck





Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

2021-03-10 Thread Greg Kurz
On Thu, 11 Mar 2021 05:15:37 +0200
Mahmoud Mandour  wrote:

> Replaced a call to qemu_mutex_lock and its respective call to
> qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> This simplifies the code by removing the call required to unlock
> and also eliminates goto paths.
> 
> Signed-off-by: Mahmoud Mandour 
> ---

Nice cleanup. Thanks !

Acked-by: Greg Kurz 

>  hw/9pfs/9p-synth.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index 7eb210ffa8..473ef914b0 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>  if (!parent) {
>  parent = _root;
>  }
> -qemu_mutex_lock(_mutex);
> +QEMU_LOCK_GUARD(_mutex);
>  QLIST_FOREACH(tmp, >child, sibling) {
>  if (!strcmp(tmp->name, name)) {
>  ret = EEXIST;
> -goto err_out;
> +return ret;
>  }
>  }
>  /* Add the name */
> @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
>node->attr, node->attr->inode);
>  *result = node;
>  ret = 0;
> -err_out:
> -qemu_mutex_unlock(_mutex);
>  return ret;
>  }
>  
> @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int 
> mode,
>  parent = _root;
>  }
>  
> -qemu_mutex_lock(_mutex);
> +QEMU_LOCK_GUARD(_mutex);
>  QLIST_FOREACH(tmp, >child, sibling) {
>  if (!strcmp(tmp->name, name)) {
>  ret = EEXIST;
> -goto err_out;
> +return ret;
>  }
>  }
>  /* Add file type and remove write bits */
> @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int 
> mode,
>  pstrcpy(node->name, sizeof(node->name), name);
>  QLIST_INSERT_HEAD_RCU(>child, node, sibling);
>  ret = 0;
> -err_out:
> -qemu_mutex_unlock(_mutex);
>  return ret;
>  }
>