mbox series

[0/9] relay: cleanup and const callbacks, take 2

Message ID cover.1606153547.git.jani.nikula@intel.com
Headers show
Series relay: cleanup and const callbacks, take 2 | expand

Message

Jani Nikula Nov. 23, 2020, 5:59 p.m. UTC 
This is v2 of [1], with a number of cleanups added first based on
Christoph's feedback, making the actual constness patch much smaller and
cleaner.

I don't know who actually maintains relay, it's not in MAINTAINERS -
Cc'd Andrew just in case.

I'd think it would be simplest to queue patches 5-9 via whichever tree
the relay patches get merged. They're all one-liners so neglible
conflict potential.

BR,
Jani.


[1] http://lore.kernel.org/r/20201118165320.26829-1-jani.nikula@intel.com


Cc: linux-block@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>
Cc: ath11k@lists.infradead.org
Cc: ath10k@lists.infradead.org
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>


Jani Nikula (9):
  relay: remove unused buf_mapped and buf_unmapped callbacks
  relay: require non-NULL callbacks in relay_open()
  relay: make create_buf_file and remove_buf_file callbacks mandatory
  relay: allow the use of const callback structs
  drm/i915: make relay callbacks const
  ath10k: make relay callbacks const
  ath11k: make relay callbacks const
  ath9k: make relay callbacks const
  blktrace: make relay callbacks const

 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c    |   2 +-
 drivers/net/wireless/ath/ath10k/spectral.c    |   2 +-
 drivers/net/wireless/ath/ath11k/spectral.c    |   2 +-
 .../net/wireless/ath/ath9k/common-spectral.c  |   2 +-
 include/linux/relay.h                         |  29 ++---
 kernel/relay.c                                | 107 +++---------------
 kernel/trace/blktrace.c                       |   2 +-
 7 files changed, 26 insertions(+), 120 deletions(-)

Comments

Kalle Valo Nov. 23, 2020, 6:06 p.m. UTC | #1 
Jani Nikula <jani.nikula@intel.com> writes:

> This is v2 of [1], with a number of cleanups added first based on
> Christoph's feedback, making the actual constness patch much smaller and
> cleaner.
>
> I don't know who actually maintains relay, it's not in MAINTAINERS -
> Cc'd Andrew just in case.
>
> I'd think it would be simplest to queue patches 5-9 via whichever tree
> the relay patches get merged. They're all one-liners so neglible
> conflict potential.
>
> BR,
> Jani.
>
>
> [1] http://lore.kernel.org/r/20201118165320.26829-1-jani.nikula@intel.com
>
>
> Cc: linux-block@vger.kernel.org
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: ath11k@lists.infradead.org
> Cc: ath10k@lists.infradead.org
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: linux-wireless@vger.kernel.org
> Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
>
>
> Jani Nikula (9):
>   relay: remove unused buf_mapped and buf_unmapped callbacks
>   relay: require non-NULL callbacks in relay_open()
>   relay: make create_buf_file and remove_buf_file callbacks mandatory
>   relay: allow the use of const callback structs
>   drm/i915: make relay callbacks const
>   ath10k: make relay callbacks const
>   ath11k: make relay callbacks const
>   ath9k: make relay callbacks const
>   blktrace: make relay callbacks const

For ath9k, ath10k & ath11k:

Acked-by: Kalle Valo <kvalo@codeaurora.org>
Christoph Hellwig Nov. 24, 2020, 9:42 a.m. UTC | #2 
> +/* subbuf_start callback wrapper */

> +static int cb_subbuf_start(struct rchan_buf *buf, void *subbuf,

> +			   void *prev_subbuf, size_t prev_padding)


I don't think the comment adds any information over just looking at the
function and the two callers.  I'd also name it relay_subbuf_start
instead of the cb_ prefix not used anywhere else in the file.


>  {

> +	if (buf->chan->cb->subbuf_start)

> +		return buf->chan->cb->subbuf_start(buf, subbuf,

> +						   prev_subbuf, prev_padding);

> +

>  	if (relay_buf_full(buf))

>  		return 0;


This could also be simplified a bit more to:

	if (!buf->chan->cb->subbuf_start)
		return !relay_buf_full(buf);
	return buf->chan->cb->subbuf_start(buf, subbuf, prev_subbuf,
					   prev_padding);

Otherwise this looks good to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jani Nikula Nov. 24, 2020, 11:56 a.m. UTC | #3 
On Tue, 24 Nov 2020, Christoph Hellwig <hch@infradead.org> wrote:
> Otherwise this looks good to me:


v3 sent.

> Reviewed-by: Christoph Hellwig <hch@lst.de>


Thanks for the reviews, appreciated.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center