linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] relay: allow the use of const callback structs
@ 2020-11-18 16:53 Jani Nikula
  2020-11-18 16:53 ` [PATCH 2/6] drm/i915: make relay callbacks const Jani Nikula
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: jani.nikula, intel-gfx, linux-block, Jens Axboe, ath11k, ath10k,
	Kalle Valo, linux-wireless, QCA ath9k Development

None of the relay users require the use of mutable structs for
callbacks, however the relay code does. Instead of assigning default
callbacks when there is none, add callback wrappers to conditionally
call the client callbacks if available, and fall back to default
behaviour (typically no-op) 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
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 include/linux/relay.h |   4 +-
 kernel/relay.c        | 182 +++++++++++++++++++-----------------------
 2 files changed, 86 insertions(+), 100 deletions(-)

diff --git a/include/linux/relay.h b/include/linux/relay.h
index e13a333e7c37..7333909df65a 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, may be NULL */
 	struct kref kref;		/* channel refcount */
 	void *private_data;		/* for user-defined data */
 	size_t last_toobig;		/* tried to log event > subbuf size */
@@ -170,7 +170,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 b08d936d5fa7..c53676f2d10f 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -27,13 +27,86 @@
 static DEFINE_MUTEX(relay_channels_mutex);
 static LIST_HEAD(relay_channels);
 
+/*
+ * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
+ * to default behaviour.
+ */
+
+/*
+ * subbuf_start() callback.
+ */
+static int cb_subbuf_start(const struct rchan_callbacks *cb,
+			   struct rchan_buf *buf,
+			   void *subbuf,
+			   void *prev_subbuf,
+			   size_t prev_padding)
+{
+	if (cb && cb->subbuf_start)
+		return cb->subbuf_start(buf, subbuf, prev_subbuf, prev_padding);
+
+	if (relay_buf_full(buf))
+		return 0;
+
+	return 1;
+}
+
+/*
+ * buf_mapped() callback.
+ */
+static void cb_buf_mapped(const struct rchan_callbacks *cb,
+			  struct rchan_buf *buf,
+			  struct file *filp)
+{
+	if (cb && cb->buf_mapped)
+		cb->buf_mapped(buf, filp);
+}
+
+/*
+ * buf_unmapped() callback.
+ */
+static void cb_buf_unmapped(const struct rchan_callbacks *cb,
+			    struct rchan_buf *buf,
+			    struct file *filp)
+{
+	if (cb && cb->buf_unmapped)
+		cb->buf_unmapped(buf, filp);
+}
+
+/*
+ * create_buf_file_create() callback.
+ */
+static struct dentry *cb_create_buf_file(const struct rchan_callbacks *cb,
+					 const char *filename,
+					 struct dentry *parent,
+					 umode_t mode,
+					 struct rchan_buf *buf,
+					 int *is_global)
+{
+	if (cb && cb->create_buf_file)
+		return cb->create_buf_file(filename, parent, mode, buf, is_global);
+
+	return NULL;
+}
+
+/*
+ * remove_buf_file() callback.
+ */
+static int cb_remove_buf_file(const struct rchan_callbacks *cb,
+			      struct dentry *dentry)
+{
+	if (cb && cb->remove_buf_file)
+		return cb->remove_buf_file(dentry);
+
+	return -EINVAL;
+}
+
 /*
  * close() vm_op implementation for relay file mapping.
  */
 static void relay_file_mmap_close(struct vm_area_struct *vma)
 {
 	struct rchan_buf *buf = vma->vm_private_data;
-	buf->chan->cb->buf_unmapped(buf, vma->vm_file);
+	cb_buf_unmapped(buf->chan->cb, buf, vma->vm_file);
 }
 
 /*
@@ -107,7 +180,7 @@ static int relay_mmap_buf(struct rchan_buf *buf, struct vm_area_struct *vma)
 	vma->vm_ops = &relay_file_mmap_ops;
 	vma->vm_flags |= VM_DONTEXPAND;
 	vma->vm_private_data = buf;
-	buf->chan->cb->buf_mapped(buf, filp);
+	cb_buf_mapped(buf->chan->cb, buf, filp);
 
 	return 0;
 }
@@ -264,70 +337,6 @@ 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)
-{
-	if (relay_buf_full(buf))
-		return 0;
-
-	return 1;
-}
-
-/*
- * buf_mapped() default callback.  Does nothing.
- */
-static void buf_mapped_default_callback(struct rchan_buf *buf,
-					struct file *filp)
-{
-}
-
-/*
- * buf_unmapped() default callback.  Does nothing.
- */
-static void buf_unmapped_default_callback(struct rchan_buf *buf,
-					  struct file *filp)
-{
-}
-
-/*
- * create_buf_file_create() default callback.  Does nothing.
- */
-static struct dentry *create_buf_file_default_callback(const char *filename,
-						       struct dentry *parent,
-						       umode_t mode,
-						       struct rchan_buf *buf,
-						       int *is_global)
-{
-	return NULL;
-}
-
-/*
- * remove_buf_file() default callback.  Does nothing.
- */
-static int remove_buf_file_default_callback(struct dentry *dentry)
-{
-	return -EINVAL;
-}
-
-/* relay channel default callbacks */
-static struct rchan_callbacks default_channel_callbacks = {
-	.subbuf_start = subbuf_start_default_callback,
-	.buf_mapped = buf_mapped_default_callback,
-	.buf_unmapped = buf_unmapped_default_callback,
-	.create_buf_file = create_buf_file_default_callback,
-	.remove_buf_file = remove_buf_file_default_callback,
-};
-
 /**
  *	wakeup_readers - wake up readers waiting on a channel
  *	@work: contains the channel buffer
@@ -371,7 +380,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->chan->cb, buf, buf->data, NULL, 0);
 }
 
 /**
@@ -426,9 +435,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
 	snprintf(tmpname, NAME_MAX, "%s%d", chan->base_filename, cpu);
 
 	/* Create file in fs */
-	dentry = chan->cb->create_buf_file(tmpname, chan->parent,
-					   S_IRUSR, buf,
-					   &chan->is_global);
+	dentry = cb_create_buf_file(chan->cb, tmpname, chan->parent,
+				    S_IRUSR, buf, &chan->is_global);
 	if (IS_ERR(dentry))
 		dentry = NULL;
 
@@ -461,9 +469,8 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
 		relay_set_buf_dentry(buf, dentry);
 	} else {
 		/* Only retrieve global info, nothing more, nothing less */
-		dentry = chan->cb->create_buf_file(NULL, NULL,
-						   S_IRUSR, buf,
-						   &chan->is_global);
+		dentry = cb_create_buf_file(chan->cb, NULL, NULL,
+					    S_IRUSR, buf, &chan->is_global);
 		if (IS_ERR_OR_NULL(dentry))
 			goto free_buf;
 	}
@@ -495,31 +502,10 @@ static void relay_close_buf(struct rchan_buf *buf)
 {
 	buf->finalized = 1;
 	irq_work_sync(&buf->wakeup_work);
-	buf->chan->cb->remove_buf_file(buf->dentry);
+	cb_remove_buf_file(buf->chan->cb, buf->dentry);
 	kref_put(&buf->kref, relay_remove_buf);
 }
 
-static void setup_callbacks(struct rchan *chan,
-				   struct rchan_callbacks *cb)
-{
-	if (!cb) {
-		chan->cb = &default_channel_callbacks;
-		return;
-	}
-
-	if (!cb->subbuf_start)
-		cb->subbuf_start = subbuf_start_default_callback;
-	if (!cb->buf_mapped)
-		cb->buf_mapped = buf_mapped_default_callback;
-	if (!cb->buf_unmapped)
-		cb->buf_unmapped = buf_unmapped_default_callback;
-	if (!cb->create_buf_file)
-		cb->create_buf_file = create_buf_file_default_callback;
-	if (!cb->remove_buf_file)
-		cb->remove_buf_file = remove_buf_file_default_callback;
-	chan->cb = cb;
-}
-
 int relay_prepare_cpu(unsigned int cpu)
 {
 	struct rchan *chan;
@@ -565,7 +551,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;
@@ -597,7 +583,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);
@@ -780,7 +766,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->chan->cb, buf, new, old, buf->prev_padding)) {
 		buf->offset = buf->chan->subbuf_size + 1;
 		return 0;
 	}
-- 
2.20.1


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

* [PATCH 2/6] drm/i915: make relay callbacks const
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
@ 2020-11-18 16:53 ` Jani Nikula
  2020-11-18 16:53 ` [PATCH 3/6] ath10k: " Jani Nikula
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: jani.nikula, intel-gfx

Now that relay_open() accepts const callbacks, make relay callbacks
const.

Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index 9bbe8a795cb8..c92f2c056db4 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -134,7 +134,7 @@ static int remove_buf_file_callback(struct dentry *dentry)
 }
 
 /* relay channel callbacks */
-static struct rchan_callbacks relay_callbacks = {
+static const struct rchan_callbacks relay_callbacks = {
 	.subbuf_start = subbuf_start_callback,
 	.create_buf_file = create_buf_file_callback,
 	.remove_buf_file = remove_buf_file_callback,
-- 
2.20.1


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

* [PATCH 3/6] ath10k: make relay callbacks const
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
  2020-11-18 16:53 ` [PATCH 2/6] drm/i915: make relay callbacks const Jani Nikula
@ 2020-11-18 16:53 ` Jani Nikula
  2020-11-18 17:13   ` Kalle Valo
  2020-11-18 16:53 ` [PATCH 4/6] ath11k: " Jani Nikula
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: jani.nikula, intel-gfx, Kalle Valo, ath10k

Now that relay_open() accepts const callbacks, make relay callbacks
const.

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: ath10k@lists.infradead.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


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

* [PATCH 4/6] ath11k: make relay callbacks const
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
  2020-11-18 16:53 ` [PATCH 2/6] drm/i915: make relay callbacks const Jani Nikula
  2020-11-18 16:53 ` [PATCH 3/6] ath10k: " Jani Nikula
@ 2020-11-18 16:53 ` Jani Nikula
  2020-11-19  9:31   ` Jani Nikula
  2020-11-18 16:53 ` [PATCH 5/6] ath9k: " Jani Nikula
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: jani.nikula, intel-gfx, Kalle Valo, ath11k

Now that relay_open() accepts const callbacks, make relay callbacks
const.

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: ath11k@lists.infradead.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
index ac2a8cfdc1c0..1afe67759659 100644
--- a/drivers/net/wireless/ath/ath11k/spectral.c
+++ b/drivers/net/wireless/ath/ath11k/spectral.c
@@ -148,7 +148,7 @@ static int remove_buf_file_handler(struct dentry *dentry)
 	return 0;
 }
 
-static struct rchan_callbacks rfs_scan_cb = {
+static const struct rchan_callbacks rfs_scan_cb = {
 	.create_buf_file = create_buf_file_handler,
 	.remove_buf_file = remove_buf_file_handler,
 };
-- 
2.20.1


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

* [PATCH 5/6] ath9k: make relay callbacks const
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
                   ` (2 preceding siblings ...)
  2020-11-18 16:53 ` [PATCH 4/6] ath11k: " Jani Nikula
@ 2020-11-18 16:53 ` Jani Nikula
  2020-11-18 17:03   ` Kalle Valo
  2020-11-18 16:53 ` [PATCH 6/6] blktrace: " Jani Nikula
  2020-11-19  8:11 ` [PATCH 1/6] relay: allow the use of const callback structs Christoph Hellwig
  5 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: jani.nikula, intel-gfx, Kalle Valo, QCA ath9k Development,
	linux-wireless

Now that relay_open() accepts const callbacks, make relay callbacks
const.

Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
Cc: linux-wireless@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/net/wireless/ath/ath9k/common-spectral.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index 21191955a7c1..e055adfb5361 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -1053,7 +1053,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


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

* [PATCH 6/6] blktrace: make relay callbacks const
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
                   ` (3 preceding siblings ...)
  2020-11-18 16:53 ` [PATCH 5/6] ath9k: " Jani Nikula
@ 2020-11-18 16:53 ` Jani Nikula
  2020-11-19  8:11 ` [PATCH 1/6] relay: allow the use of const callback structs Christoph Hellwig
  5 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2020-11-18 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: jani.nikula, intel-gfx, Jens Axboe, linux-block

Now that relay_open() accepts const callbacks, make relay callbacks
const.

Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 kernel/trace/blktrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index f1022945e346..b5c4b9ade960 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -449,7 +449,7 @@ static struct dentry *blk_create_buf_file_callback(const char *filename,
 					&relay_file_operations);
 }
 
-static struct rchan_callbacks blk_relay_callbacks = {
+static const struct rchan_callbacks blk_relay_callbacks = {
 	.subbuf_start		= blk_subbuf_start_callback,
 	.create_buf_file	= blk_create_buf_file_callback,
 	.remove_buf_file	= blk_remove_buf_file_callback,
-- 
2.20.1


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

* Re: [PATCH 5/6] ath9k: make relay callbacks const
  2020-11-18 16:53 ` [PATCH 5/6] ath9k: " Jani Nikula
@ 2020-11-18 17:03   ` Kalle Valo
  2020-11-18 17:15     ` Kalle Valo
  0 siblings, 1 reply; 14+ messages in thread
From: Kalle Valo @ 2020-11-18 17:03 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, QCA ath9k Development, linux-wireless

Jani Nikula <jani.nikula@intel.com> writes:

> Now that relay_open() accepts const callbacks, make relay callbacks
> const.
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
> Cc: linux-wireless@vger.kernel.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Can I take this to my ath tree or what's the plan?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: [PATCH 3/6] ath10k: make relay callbacks const
  2020-11-18 16:53 ` [PATCH 3/6] ath10k: " Jani Nikula
@ 2020-11-18 17:13   ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2020-11-18 17:13 UTC (permalink / raw)
  To: Jani Nikula; +Cc: linux-kernel, intel-gfx, ath10k

Jani Nikula <jani.nikula@intel.com> writes:

> Now that relay_open() accepts const callbacks, make relay callbacks
> const.
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: ath10k@lists.infradead.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I assume this goes via some other tree:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: [PATCH 5/6] ath9k: make relay callbacks const
  2020-11-18 17:03   ` Kalle Valo
@ 2020-11-18 17:15     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2020-11-18 17:15 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, QCA ath9k Development, linux-wireless

Kalle Valo <kvalo@codeaurora.org> writes:

> Jani Nikula <jani.nikula@intel.com> writes:
>
>> Now that relay_open() accepts const callbacks, make relay callbacks
>> const.
>>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
>> Cc: linux-wireless@vger.kernel.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Can I take this to my ath tree or what's the plan?

Ah, saw patch 1 only now. So I assume this goes via some other tree:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: [PATCH 1/6] relay: allow the use of const callback structs
  2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
                   ` (4 preceding siblings ...)
  2020-11-18 16:53 ` [PATCH 6/6] blktrace: " Jani Nikula
@ 2020-11-19  8:11 ` Christoph Hellwig
  2020-11-19  8:11   ` Christoph Hellwig
  2020-11-23 18:01   ` Jani Nikula
  5 siblings, 2 replies; 14+ messages in thread
From: Christoph Hellwig @ 2020-11-19  8:11 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, linux-block, Jens Axboe, ath11k, ath10k,
	Kalle Valo, linux-wireless, QCA ath9k Development

> +/*
> + * rchan_callback wrappers. Call the callbacks if available, otherwise fall back
> + * to default behaviour.
> + */

This adds an overly long line.  That being said this behavior is pretty
normal for kernel APIs, so I'm not even sure we need it at all.

> +
> +/*
> + * subbuf_start() callback.
> + */

and this one is for sure completley useless.  Same for all the other
similar ones.


But taking one step back:  All instances implement create_buf_file
and remove_buf_file, which makes sense as that is the prime aim
of these methods.  So there is no point in making those optional.
subbuf_start_callback is overriden by two instances, so making that
optional totally makes sense.  buf_mapped and buf_unmapped are
never overriden, so they should be removed entirely.

More importantly there is no case that passes a NULL rchan_callbacks,
which makes complete sense as it wouldn't even create a file.  So
remove that case as well and just replace it with a sanity check in
relay_open().

Please also add a patch to mark all rchan_callbacks instances const
while you're at it.

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

* Re: [PATCH 1/6] relay: allow the use of const callback structs
  2020-11-19  8:11 ` [PATCH 1/6] relay: allow the use of const callback structs Christoph Hellwig
@ 2020-11-19  8:11   ` Christoph Hellwig
  2020-11-23 18:01   ` Jani Nikula
  1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2020-11-19  8:11 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, intel-gfx, linux-block, Jens Axboe, ath11k, ath10k,
	Kalle Valo, linux-wireless, QCA ath9k Development

On Thu, Nov 19, 2020 at 08:11:20AM +0000, Christoph Hellwig wrote:
> Please also add a patch to mark all rchan_callbacks instances const
> while you're at it.

Oops, I just noticed you actually sent that one.

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

* Re: [PATCH 4/6] ath11k: make relay callbacks const
  2020-11-18 16:53 ` [PATCH 4/6] ath11k: " Jani Nikula
@ 2020-11-19  9:31   ` Jani Nikula
  2020-11-19 11:40     ` Kalle Valo
  0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2020-11-19  9:31 UTC (permalink / raw)
  To: linux-kernel; +Cc: intel-gfx, Kalle Valo, ath11k

On Wed, 18 Nov 2020, Jani Nikula <jani.nikula@intel.com> wrote:
> Now that relay_open() accepts const callbacks, make relay callbacks
> const.
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: ath11k@lists.infradead.org
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Kalle, thanks for the acks on the other two ath patches - can I have
your ack on this one too please?

BR,
Jani.

> ---
>  drivers/net/wireless/ath/ath11k/spectral.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/spectral.c b/drivers/net/wireless/ath/ath11k/spectral.c
> index ac2a8cfdc1c0..1afe67759659 100644
> --- a/drivers/net/wireless/ath/ath11k/spectral.c
> +++ b/drivers/net/wireless/ath/ath11k/spectral.c
> @@ -148,7 +148,7 @@ static int remove_buf_file_handler(struct dentry *dentry)
>  	return 0;
>  }
>  
> -static struct rchan_callbacks rfs_scan_cb = {
> +static const struct rchan_callbacks rfs_scan_cb = {
>  	.create_buf_file = create_buf_file_handler,
>  	.remove_buf_file = remove_buf_file_handler,
>  };

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH 4/6] ath11k: make relay callbacks const
  2020-11-19  9:31   ` Jani Nikula
@ 2020-11-19 11:40     ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2020-11-19 11:40 UTC (permalink / raw)
  To: Jani Nikula; +Cc: linux-kernel, intel-gfx, ath11k

Jani Nikula <jani.nikula@intel.com> writes:

> On Wed, 18 Nov 2020, Jani Nikula <jani.nikula@intel.com> wrote:
>> Now that relay_open() accepts const callbacks, make relay callbacks
>> const.
>>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: ath11k@lists.infradead.org
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Kalle, thanks for the acks on the other two ath patches - can I have
> your ack on this one too please?

Oops, missed that:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: [PATCH 1/6] relay: allow the use of const callback structs
  2020-11-19  8:11 ` [PATCH 1/6] relay: allow the use of const callback structs Christoph Hellwig
  2020-11-19  8:11   ` Christoph Hellwig
@ 2020-11-23 18:01   ` Jani Nikula
  1 sibling, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2020-11-23 18:01 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-kernel, intel-gfx, linux-block, Jens Axboe, ath11k, ath10k,
	Kalle Valo, linux-wireless, QCA ath9k Development

On Thu, 19 Nov 2020, Christoph Hellwig <hch@infradead.org> wrote:
> But taking one step back:  All instances implement create_buf_file
> and remove_buf_file, which makes sense as that is the prime aim
> of these methods.  So there is no point in making those optional.
> subbuf_start_callback is overriden by two instances, so making that
> optional totally makes sense.  buf_mapped and buf_unmapped are
> never overriden, so they should be removed entirely.
>
> More importantly there is no case that passes a NULL rchan_callbacks,
> which makes complete sense as it wouldn't even create a file.  So
> remove that case as well and just replace it with a sanity check in
> relay_open().

Many thanks for the feedback; sent v2 [1].

BR,
Jani.


[1] http://lore.kernel.org/r/cover.1606153547.git.jani.nikula@intel.com


-- 
Jani Nikula, Intel Open Source Graphics Center

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

end of thread, other threads:[~2020-11-23 18:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-18 16:53 [PATCH 1/6] relay: allow the use of const callback structs Jani Nikula
2020-11-18 16:53 ` [PATCH 2/6] drm/i915: make relay callbacks const Jani Nikula
2020-11-18 16:53 ` [PATCH 3/6] ath10k: " Jani Nikula
2020-11-18 17:13   ` Kalle Valo
2020-11-18 16:53 ` [PATCH 4/6] ath11k: " Jani Nikula
2020-11-19  9:31   ` Jani Nikula
2020-11-19 11:40     ` Kalle Valo
2020-11-18 16:53 ` [PATCH 5/6] ath9k: " Jani Nikula
2020-11-18 17:03   ` Kalle Valo
2020-11-18 17:15     ` Kalle Valo
2020-11-18 16:53 ` [PATCH 6/6] blktrace: " Jani Nikula
2020-11-19  8:11 ` [PATCH 1/6] relay: allow the use of const callback structs Christoph Hellwig
2020-11-19  8:11   ` Christoph Hellwig
2020-11-23 18:01   ` Jani Nikula

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).