> +/* 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 <[email protected]>
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: [email protected]
Cc: Jens Axboe <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Kalle Valo <[email protected]>
Cc: [email protected]
Cc: QCA ath9k Development <[email protected]>
Cc: [email protected]
Cc: Christoph Hellwig <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
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
On Tue, 24 Nov 2020, Christoph Hellwig <[email protected]> wrote:
> Otherwise this looks good to me:
v3 sent.
> Reviewed-by: Christoph Hellwig <[email protected]>
Thanks for the reviews, appreciated.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center