* [PATCH 0/9] relay: cleanup and const callbacks, take 2
@ 2020-11-23 17:59 Jani Nikula
2020-11-23 17:59 ` [PATCH 4/9] relay: allow the use of const callback structs Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jani Nikula @ 2020-11-23 17:59 UTC (permalink / raw)
To: linux-kernel
Cc: Jens Axboe, linux-block, jani.nikula, intel-gfx, linux-wireless,
QCA ath9k Development, ath10k, Christoph Hellwig, Andrew Morton,
ath11k, Kalle Valo
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(-)
--
2.20.1
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 4/9] relay: allow the use of const callback structs 2020-11-23 17:59 [PATCH 0/9] relay: cleanup and const callbacks, take 2 Jani Nikula @ 2020-11-23 17:59 ` Jani Nikula 2020-11-24 9:42 ` Christoph Hellwig 2020-11-23 17:59 ` [PATCH 6/9] ath10k: make relay callbacks const Jani Nikula 2020-11-23 18:06 ` [PATCH 0/9] relay: cleanup and const callbacks, take 2 Kalle Valo 2 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2020-11-23 17:59 UTC (permalink / raw) To: linux-kernel Cc: Jens Axboe, linux-block, jani.nikula, intel-gfx, linux-wireless, QCA ath9k Development, ath10k, Christoph Hellwig, Andrew Morton, ath11k, Kalle Valo None of the relay users require the use of mutable structs for callbacks, however the relay code does. Instead of assigning the default callback for subbuf_start, add a wrapper to conditionally call the client callback if available, and fall back to default behaviour otherwise. This lets all relay users make their struct rchan_callbacks const data. 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> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- v2: Simplify after nuking some callbacks and making some others mandatory in previous patches, as per Christoph's review comments. I thought about adding wrappers for the now-mandatory create_buf_file and remove_buf_file as well, for consistency, but ended up leaving them out. --- include/linux/relay.h | 4 ++-- kernel/relay.c | 35 +++++++++++------------------------ 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/include/linux/relay.h b/include/linux/relay.h index 99d024475ba5..72b876dd5cb8 100644 --- a/include/linux/relay.h +++ b/include/linux/relay.h @@ -62,7 +62,7 @@ struct rchan size_t subbuf_size; /* sub-buffer size */ size_t n_subbufs; /* number of sub-buffers per buffer */ size_t alloc_size; /* total buffer size allocated */ - struct rchan_callbacks *cb; /* client callbacks */ + const struct rchan_callbacks *cb; /* client callbacks */ struct kref kref; /* channel refcount */ void *private_data; /* for user-defined data */ size_t last_toobig; /* tried to log event > subbuf size */ @@ -157,7 +157,7 @@ struct rchan *relay_open(const char *base_filename, struct dentry *parent, size_t subbuf_size, size_t n_subbufs, - struct rchan_callbacks *cb, + const struct rchan_callbacks *cb, void *private_data); extern int relay_late_setup_files(struct rchan *chan, const char *base_filename, diff --git a/kernel/relay.c b/kernel/relay.c index dd4ec4ec07f3..02bdba5372cb 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -252,19 +252,14 @@ EXPORT_SYMBOL_GPL(relay_buf_full); * High-level relay kernel API and associated functions. */ -/* - * rchan_callback implementations defining default channel behavior. Used - * in place of corresponding NULL values in client callback struct. - */ - -/* - * subbuf_start() default callback. Does nothing. - */ -static int subbuf_start_default_callback (struct rchan_buf *buf, - void *subbuf, - void *prev_subbuf, - size_t prev_padding) +/* subbuf_start callback wrapper */ +static int cb_subbuf_start(struct rchan_buf *buf, void *subbuf, + void *prev_subbuf, size_t prev_padding) { + 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; @@ -314,7 +309,7 @@ static void __relay_reset(struct rchan_buf *buf, unsigned int init) for (i = 0; i < buf->chan->n_subbufs; i++) buf->padding[i] = 0; - buf->chan->cb->subbuf_start(buf, buf->data, NULL, 0); + cb_subbuf_start(buf, buf->data, NULL, 0); } /** @@ -442,14 +437,6 @@ static void relay_close_buf(struct rchan_buf *buf) kref_put(&buf->kref, relay_remove_buf); } -static void setup_callbacks(struct rchan *chan, - struct rchan_callbacks *cb) -{ - if (!cb->subbuf_start) - cb->subbuf_start = subbuf_start_default_callback; - chan->cb = cb; -} - int relay_prepare_cpu(unsigned int cpu) { struct rchan *chan; @@ -495,7 +482,7 @@ struct rchan *relay_open(const char *base_filename, struct dentry *parent, size_t subbuf_size, size_t n_subbufs, - struct rchan_callbacks *cb, + const struct rchan_callbacks *cb, void *private_data) { unsigned int i; @@ -529,7 +516,7 @@ struct rchan *relay_open(const char *base_filename, chan->has_base_filename = 1; strlcpy(chan->base_filename, base_filename, NAME_MAX); } - setup_callbacks(chan, cb); + chan->cb = cb; kref_init(&chan->kref); mutex_lock(&relay_channels_mutex); @@ -712,7 +699,7 @@ size_t relay_switch_subbuf(struct rchan_buf *buf, size_t length) new_subbuf = buf->subbufs_produced % buf->chan->n_subbufs; new = buf->start + new_subbuf * buf->chan->subbuf_size; buf->offset = 0; - if (!buf->chan->cb->subbuf_start(buf, new, old, buf->prev_padding)) { + if (!cb_subbuf_start(buf, new, old, buf->prev_padding)) { buf->offset = buf->chan->subbuf_size + 1; return 0; } -- 2.20.1 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 4/9] relay: allow the use of const callback structs 2020-11-23 17:59 ` [PATCH 4/9] relay: allow the use of const callback structs Jani Nikula @ 2020-11-24 9:42 ` Christoph Hellwig 2020-11-24 11:54 ` [PATCH v3] " Jani Nikula 2020-11-24 11:56 ` [PATCH 4/9] " Jani Nikula 0 siblings, 2 replies; 8+ messages in thread From: Christoph Hellwig @ 2020-11-24 9:42 UTC (permalink / raw) To: Jani Nikula Cc: Jens Axboe, linux-block, intel-gfx, linux-wireless, linux-kernel, ath10k, QCA ath9k Development, Christoph Hellwig, Andrew Morton, ath11k, Kalle Valo > +/* 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> _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] relay: allow the use of const callback structs 2020-11-24 9:42 ` Christoph Hellwig @ 2020-11-24 11:54 ` Jani Nikula 2020-11-24 11:56 ` [PATCH 4/9] " Jani Nikula 1 sibling, 0 replies; 8+ messages in thread From: Jani Nikula @ 2020-11-24 11:54 UTC (permalink / raw) To: Christoph Hellwig, Jani Nikula Cc: Jens Axboe, intel-gfx, linux-wireless, linux-kernel, ath10k, QCA ath9k Development, linux-block, Andrew Morton, Christoph Hellwig, ath11k, Kalle Valo None of the relay users require the use of mutable structs for callbacks, however the relay code does. Instead of assigning the default callback for subbuf_start, add a wrapper to conditionally call the client callback if available, and fall back to default behaviour otherwise. This lets all relay users make their struct rchan_callbacks const data. 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> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- v2: Simplify after nuking some callbacks and making some others mandatory in previous patches, as per Christoph's review comments. I thought about adding wrappers for the now-mandatory create_buf_file and remove_buf_file as well, for consistency, but ended up leaving them out. v3: Rename cb_subbuf_start to relay_subbuf_start, minor finishing touches --- include/linux/relay.h | 4 ++-- kernel/relay.c | 37 ++++++++++--------------------------- 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/include/linux/relay.h b/include/linux/relay.h index 99d024475ba5..72b876dd5cb8 100644 --- a/include/linux/relay.h +++ b/include/linux/relay.h @@ -62,7 +62,7 @@ struct rchan size_t subbuf_size; /* sub-buffer size */ size_t n_subbufs; /* number of sub-buffers per buffer */ size_t alloc_size; /* total buffer size allocated */ - struct rchan_callbacks *cb; /* client callbacks */ + const struct rchan_callbacks *cb; /* client callbacks */ struct kref kref; /* channel refcount */ void *private_data; /* for user-defined data */ size_t last_toobig; /* tried to log event > subbuf size */ @@ -157,7 +157,7 @@ struct rchan *relay_open(const char *base_filename, struct dentry *parent, size_t subbuf_size, size_t n_subbufs, - struct rchan_callbacks *cb, + const struct rchan_callbacks *cb, void *private_data); extern int relay_late_setup_files(struct rchan *chan, const char *base_filename, diff --git a/kernel/relay.c b/kernel/relay.c index dd4ec4ec07f3..d1a67fbb819d 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -252,23 +252,14 @@ EXPORT_SYMBOL_GPL(relay_buf_full); * High-level relay kernel API and associated functions. */ -/* - * rchan_callback implementations defining default channel behavior. Used - * in place of corresponding NULL values in client callback struct. - */ - -/* - * subbuf_start() default callback. Does nothing. - */ -static int subbuf_start_default_callback (struct rchan_buf *buf, - void *subbuf, - void *prev_subbuf, - size_t prev_padding) +static int relay_subbuf_start(struct rchan_buf *buf, void *subbuf, + void *prev_subbuf, size_t prev_padding) { - if (relay_buf_full(buf)) - return 0; + if (!buf->chan->cb->subbuf_start) + return !relay_buf_full(buf); - return 1; + return buf->chan->cb->subbuf_start(buf, subbuf, + prev_subbuf, prev_padding); } /** @@ -314,7 +305,7 @@ static void __relay_reset(struct rchan_buf *buf, unsigned int init) for (i = 0; i < buf->chan->n_subbufs; i++) buf->padding[i] = 0; - buf->chan->cb->subbuf_start(buf, buf->data, NULL, 0); + relay_subbuf_start(buf, buf->data, NULL, 0); } /** @@ -442,14 +433,6 @@ static void relay_close_buf(struct rchan_buf *buf) kref_put(&buf->kref, relay_remove_buf); } -static void setup_callbacks(struct rchan *chan, - struct rchan_callbacks *cb) -{ - if (!cb->subbuf_start) - cb->subbuf_start = subbuf_start_default_callback; - chan->cb = cb; -} - int relay_prepare_cpu(unsigned int cpu) { struct rchan *chan; @@ -495,7 +478,7 @@ struct rchan *relay_open(const char *base_filename, struct dentry *parent, size_t subbuf_size, size_t n_subbufs, - struct rchan_callbacks *cb, + const struct rchan_callbacks *cb, void *private_data) { unsigned int i; @@ -529,7 +512,7 @@ struct rchan *relay_open(const char *base_filename, chan->has_base_filename = 1; strlcpy(chan->base_filename, base_filename, NAME_MAX); } - setup_callbacks(chan, cb); + chan->cb = cb; kref_init(&chan->kref); mutex_lock(&relay_channels_mutex); @@ -712,7 +695,7 @@ size_t relay_switch_subbuf(struct rchan_buf *buf, size_t length) new_subbuf = buf->subbufs_produced % buf->chan->n_subbufs; new = buf->start + new_subbuf * buf->chan->subbuf_size; buf->offset = 0; - if (!buf->chan->cb->subbuf_start(buf, new, old, buf->prev_padding)) { + if (!relay_subbuf_start(buf, new, old, buf->prev_padding)) { buf->offset = buf->chan->subbuf_size + 1; return 0; } -- 2.20.1 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 4/9] relay: allow the use of const callback structs 2020-11-24 9:42 ` Christoph Hellwig 2020-11-24 11:54 ` [PATCH v3] " Jani Nikula @ 2020-11-24 11:56 ` Jani Nikula 1 sibling, 0 replies; 8+ messages in thread From: Jani Nikula @ 2020-11-24 11:56 UTC (permalink / raw) To: Christoph Hellwig Cc: Jens Axboe, linux-block, intel-gfx, linux-wireless, linux-kernel, ath10k, QCA ath9k Development, Andrew Morton, ath11k, Kalle Valo 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 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 6/9] ath10k: make relay callbacks const 2020-11-23 17:59 [PATCH 0/9] relay: cleanup and const callbacks, take 2 Jani Nikula 2020-11-23 17:59 ` [PATCH 4/9] relay: allow the use of const callback structs Jani Nikula @ 2020-11-23 17:59 ` Jani Nikula 2020-11-24 9:42 ` Christoph Hellwig 2020-11-23 18:06 ` [PATCH 0/9] relay: cleanup and const callbacks, take 2 Kalle Valo 2 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2020-11-23 17:59 UTC (permalink / raw) To: linux-kernel Cc: jani.nikula, intel-gfx, ath10k, Christoph Hellwig, Andrew Morton, Kalle Valo Now that relay_open() accepts const callbacks, make relay callbacks const. Cc: Kalle Valo <kvalo@codeaurora.org> Cc: ath10k@lists.infradead.org Acked-by: Kalle Valo <kvalo@codeaurora.org> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/net/wireless/ath/ath10k/spectral.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/spectral.c b/drivers/net/wireless/ath/ath10k/spectral.c index 5db6bff5193b..68254a967ccb 100644 --- a/drivers/net/wireless/ath/ath10k/spectral.c +++ b/drivers/net/wireless/ath/ath10k/spectral.c @@ -497,7 +497,7 @@ static int remove_buf_file_handler(struct dentry *dentry) return 0; } -static struct rchan_callbacks rfs_spec_scan_cb = { +static const struct rchan_callbacks rfs_spec_scan_cb = { .create_buf_file = create_buf_file_handler, .remove_buf_file = remove_buf_file_handler, }; -- 2.20.1 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 6/9] ath10k: make relay callbacks const 2020-11-23 17:59 ` [PATCH 6/9] ath10k: make relay callbacks const Jani Nikula @ 2020-11-24 9:42 ` Christoph Hellwig 0 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2020-11-24 9:42 UTC (permalink / raw) To: Jani Nikula Cc: intel-gfx, linux-kernel, ath10k, Christoph Hellwig, Andrew Morton, Kalle Valo On Mon, Nov 23, 2020 at 07:59:26PM +0200, Jani Nikula wrote: > Now that relay_open() accepts const callbacks, make relay callbacks > const. Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/9] relay: cleanup and const callbacks, take 2 2020-11-23 17:59 [PATCH 0/9] relay: cleanup and const callbacks, take 2 Jani Nikula 2020-11-23 17:59 ` [PATCH 4/9] relay: allow the use of const callback structs Jani Nikula 2020-11-23 17:59 ` [PATCH 6/9] ath10k: make relay callbacks const Jani Nikula @ 2020-11-23 18:06 ` Kalle Valo 2 siblings, 0 replies; 8+ messages in thread From: Kalle Valo @ 2020-11-23 18:06 UTC (permalink / raw) To: Jani Nikula Cc: Jens Axboe, Christoph Hellwig, intel-gfx, linux-wireless, linux-kernel, ath10k, QCA ath9k Development, linux-block, Andrew Morton, ath11k 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> -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-24 11:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-23 17:59 [PATCH 0/9] relay: cleanup and const callbacks, take 2 Jani Nikula 2020-11-23 17:59 ` [PATCH 4/9] relay: allow the use of const callback structs Jani Nikula 2020-11-24 9:42 ` Christoph Hellwig 2020-11-24 11:54 ` [PATCH v3] " Jani Nikula 2020-11-24 11:56 ` [PATCH 4/9] " Jani Nikula 2020-11-23 17:59 ` [PATCH 6/9] ath10k: make relay callbacks const Jani Nikula 2020-11-24 9:42 ` Christoph Hellwig 2020-11-23 18:06 ` [PATCH 0/9] relay: cleanup and const callbacks, take 2 Kalle Valo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).