All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit()
       [not found] <20190329200445.28512-1-chen.zhang@intel.com>
@ 2019-04-03 20:44 ` John Snow
  2019-04-04  1:43 ` Peter Maydell
  1 sibling, 0 replies; 3+ messages in thread
From: John Snow @ 2019-04-03 20:44 UTC (permalink / raw)
  To: Zhang Chen, Dr. David Alan Gilbert, qemu-dev, Fam Zheng



On 3/29/19 4:04 PM, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
> 
> In current codes we use change_bit() to finish the job.
> 
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  include/qemu/bitmap.h |  1 -
>  include/qemu/bitops.h | 15 ---------------
>  2 files changed, 16 deletions(-)
> 
> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
> index 5c313346b9..6b71ef631c 100644
> --- a/include/qemu/bitmap.h
> +++ b/include/qemu/bitmap.h
> @@ -52,7 +52,6 @@
>   * test_bit(bit, addr)			Is bit set in *addr?
>   * test_and_set_bit(bit, addr)		Set bit and return old value
>   * test_and_clear_bit(bit, addr)	Clear bit and return old value
> - * test_and_change_bit(bit, addr)	Change bit and return old value
>   * find_first_zero_bit(addr, nbits)	Position first zero bit in *addr
>   * find_first_bit(addr, nbits)		Position first set bit in *addr
>   * find_next_zero_bit(addr, nbits, bit)	Position next zero bit in *addr >= bit
> diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
> index 3f0926cf40..1f98ffcdc0 100644
> --- a/include/qemu/bitops.h
> +++ b/include/qemu/bitops.h
> @@ -109,21 +109,6 @@ static inline int test_and_clear_bit(long nr, unsigned long *addr)
>      return (old & mask) != 0;
>  }
>  
> -/**
> - * test_and_change_bit - Change a bit and return its old value
> - * @nr: Bit to change
> - * @addr: Address to count from
> - */
> -static inline int test_and_change_bit(long nr, unsigned long *addr)
> -{
> -    unsigned long mask = BIT_MASK(nr);
> -    unsigned long *p = addr + BIT_WORD(nr);
> -    unsigned long old = *p;
> -
> -    *p = old ^ mask;
> -    return (old & mask) != 0;
> -}
> -
>  /**
>   * test_bit - Determine whether a bit is set
>   * @nr: bit number to test
> 

I personally don't see the harm in keeping this, but it is indeed
unused, so:

Reviewed-by: John Snow <jsnow@redhat.com>


As for merging other sibling functions, I guess the desire is a small
decrease in SLOC; I'm not sure if you'll run into any uses where the
changed signatures for a combined function causes issues. I am not sure
it's worth the hassle.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit()
       [not found] <20190329200445.28512-1-chen.zhang@intel.com>
  2019-04-03 20:44 ` [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit() John Snow
@ 2019-04-04  1:43 ` Peter Maydell
  2019-04-04  6:25   ` Zhang, Chen
  1 sibling, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2019-04-04  1:43 UTC (permalink / raw)
  To: Zhang Chen; +Cc: Dr. David Alan Gilbert, qemu-dev, John Snow, Fam Zheng

On Sat, 30 Mar 2019 at 03:09, Zhang Chen <chen.zhang@intel.com> wrote:
>
> From: Zhang Chen <chen.zhang@intel.com>
>
> In current codes we use change_bit() to finish the job.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  include/qemu/bitmap.h |  1 -
>  include/qemu/bitops.h | 15 ---------------
>  2 files changed, 16 deletions(-)

Do we gain anything by removing this function? IIRC these functions
are all borrowed from the Linux kernel, so keeping the same API
as the kernel does would make sense to me, even if we happen
not to use all of it right now.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit()
  2019-04-04  1:43 ` Peter Maydell
@ 2019-04-04  6:25   ` Zhang, Chen
  0 siblings, 0 replies; 3+ messages in thread
From: Zhang, Chen @ 2019-04-04  6:25 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Dr. David Alan Gilbert, qemu-dev, John Snow, Fam Zheng



> -----Original Message-----
> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> Sent: Thursday, April 4, 2019 9:43 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>; John Snow <jsnow@redhat.com>; Fam Zheng
> <fam@euphon.net>
> Subject: Re: [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function
> test_and_change_bit()
> 
> On Sat, 30 Mar 2019 at 03:09, Zhang Chen <chen.zhang@intel.com> wrote:
> >
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > In current codes we use change_bit() to finish the job.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  include/qemu/bitmap.h |  1 -
> >  include/qemu/bitops.h | 15 ---------------
> >  2 files changed, 16 deletions(-)
> 
> Do we gain anything by removing this function? IIRC these functions are all
> borrowed from the Linux kernel, so keeping the same API as the kernel does
> would make sense to me, even if we happen not to use all of it right now.
> 

Hi Peter,

OK, I agree with you.
But another question are the bitmap.h and bitops.h lack of maintenance.
Please look at this patch:
[PATCH] MAINTAINERS: Add unmaintained bitmap file to related field

Do you have any comments?

Thanks
Zhang Chen

> thanks
> -- PMM

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-04-04  6:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190329200445.28512-1-chen.zhang@intel.com>
2019-04-03 20:44 ` [Qemu-devel] [PATCH] bitops.h: Remove unused bitops function test_and_change_bit() John Snow
2019-04-04  1:43 ` Peter Maydell
2019-04-04  6:25   ` Zhang, Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.