We have largely moved away from this approach, and we have better
debugging tooling nowadays: kill it.
Link: https://lore.kernel.org/linux-doc/[email protected]/
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
---
Documentation/process/magic-number.rst | 1 -
Documentation/translations/it_IT/process/magic-number.rst | 1 -
Documentation/translations/zh_CN/process/magic-number.rst | 1 -
Documentation/translations/zh_TW/process/magic-number.rst | 1 -
fs/coda/cnode.c | 2 +-
fs/coda/coda_fs_i.h | 2 --
fs/coda/file.c | 1 -
7 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
index 18f8b1e3a993..335169e43be1 100644
--- a/Documentation/process/magic-number.rst
+++ b/Documentation/process/magic-number.rst
@@ -74,7 +74,6 @@ FASYNC_MAGIC 0x4601 fasync_struct ``include/linux/
SLIP_MAGIC 0x5302 slip ``drivers/net/slip.h``
HDLCDRV_MAGIC 0x5ac6e778 hdlcdrv_state ``include/linux/hdlcdrv.h``
KV_MAGIC 0x5f4b565f kernel_vars_s ``arch/mips/include/asm/sn/klkernvars.h``
-CODA_MAGIC 0xC0DAC0DA coda_file_info ``fs/coda/coda_fs_i.h``
CCB_MAGIC 0xf2691ad2 ccb ``drivers/scsi/ncr53c8xx.c``
QUEUE_MAGIC_FREE 0xf7e1c9a3 queue_entry ``drivers/scsi/arm/queue.c``
QUEUE_MAGIC_USED 0xf7e1cc33 queue_entry ``drivers/scsi/arm/queue.c``
diff --git a/Documentation/translations/it_IT/process/magic-number.rst b/Documentation/translations/it_IT/process/magic-number.rst
index 827167b18f15..699b681088ac 100644
--- a/Documentation/translations/it_IT/process/magic-number.rst
+++ b/Documentation/translations/it_IT/process/magic-number.rst
@@ -80,7 +80,6 @@ FASYNC_MAGIC 0x4601 fasync_struct ``include/linux/
SLIP_MAGIC 0x5302 slip ``drivers/net/slip.h``
HDLCDRV_MAGIC 0x5ac6e778 hdlcdrv_state ``include/linux/hdlcdrv.h``
KV_MAGIC 0x5f4b565f kernel_vars_s ``arch/mips/include/asm/sn/klkernvars.h``
-CODA_MAGIC 0xC0DAC0DA coda_file_info ``fs/coda/coda_fs_i.h``
CCB_MAGIC 0xf2691ad2 ccb ``drivers/scsi/ncr53c8xx.c``
QUEUE_MAGIC_FREE 0xf7e1c9a3 queue_entry ``drivers/scsi/arm/queue.c``
QUEUE_MAGIC_USED 0xf7e1cc33 queue_entry ``drivers/scsi/arm/queue.c``
diff --git a/Documentation/translations/zh_CN/process/magic-number.rst b/Documentation/translations/zh_CN/process/magic-number.rst
index 9553475e9867..d1ede86944f1 100644
--- a/Documentation/translations/zh_CN/process/magic-number.rst
+++ b/Documentation/translations/zh_CN/process/magic-number.rst
@@ -63,7 +63,6 @@ FASYNC_MAGIC 0x4601 fasync_struct ``include/linux/
SLIP_MAGIC 0x5302 slip ``drivers/net/slip.h``
HDLCDRV_MAGIC 0x5ac6e778 hdlcdrv_state ``include/linux/hdlcdrv.h``
KV_MAGIC 0x5f4b565f kernel_vars_s ``arch/mips/include/asm/sn/klkernvars.h``
-CODA_MAGIC 0xC0DAC0DA coda_file_info ``fs/coda/coda_fs_i.h``
CCB_MAGIC 0xf2691ad2 ccb ``drivers/scsi/ncr53c8xx.c``
QUEUE_MAGIC_FREE 0xf7e1c9a3 queue_entry ``drivers/scsi/arm/queue.c``
QUEUE_MAGIC_USED 0xf7e1cc33 queue_entry ``drivers/scsi/arm/queue.c``
diff --git a/Documentation/translations/zh_TW/process/magic-number.rst b/Documentation/translations/zh_TW/process/magic-number.rst
index 8a64f56ae267..1dd01f1e1c17 100644
--- a/Documentation/translations/zh_TW/process/magic-number.rst
+++ b/Documentation/translations/zh_TW/process/magic-number.rst
@@ -66,7 +66,6 @@ FASYNC_MAGIC 0x4601 fasync_struct ``include/linux/
SLIP_MAGIC 0x5302 slip ``drivers/net/slip.h``
HDLCDRV_MAGIC 0x5ac6e778 hdlcdrv_state ``include/linux/hdlcdrv.h``
KV_MAGIC 0x5f4b565f kernel_vars_s ``arch/mips/include/asm/sn/klkernvars.h``
-CODA_MAGIC 0xC0DAC0DA coda_file_info ``fs/coda/coda_fs_i.h``
CCB_MAGIC 0xf2691ad2 ccb ``drivers/scsi/ncr53c8xx.c``
QUEUE_MAGIC_FREE 0xf7e1c9a3 queue_entry ``drivers/scsi/arm/queue.c``
QUEUE_MAGIC_USED 0xf7e1cc33 queue_entry ``drivers/scsi/arm/queue.c``
diff --git a/fs/coda/cnode.c b/fs/coda/cnode.c
index 62a3d2565c26..e217cca338bd 100644
--- a/fs/coda/cnode.c
+++ b/fs/coda/cnode.c
@@ -157,7 +157,7 @@ struct coda_file_info *coda_ftoc(struct file *file)
{
struct coda_file_info *cfi = file->private_data;
- BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC);
+ BUG_ON(!cfi);
return cfi;
diff --git a/fs/coda/coda_fs_i.h b/fs/coda/coda_fs_i.h
index 1763ff95d865..9e4b54dbe7d7 100644
--- a/fs/coda/coda_fs_i.h
+++ b/fs/coda/coda_fs_i.h
@@ -35,9 +35,7 @@ struct coda_inode_info {
/*
* coda fs file private data
*/
-#define CODA_MAGIC 0xC0DAC0DA
struct coda_file_info {
- int cfi_magic; /* magic number */
struct file *cfi_container; /* container file for this cnode */
unsigned int cfi_mapcount; /* nr of times this file is mapped */
bool cfi_access_intent; /* is access intent supported */
diff --git a/fs/coda/file.c b/fs/coda/file.c
index 3f3c81e6b1ab..c23f846bf206 100644
--- a/fs/coda/file.c
+++ b/fs/coda/file.c
@@ -222,7 +222,6 @@ int coda_open(struct inode *coda_inode, struct file *coda_file)
host_file->f_flags |= coda_file->f_flags & (O_APPEND | O_SYNC);
- cfi->cfi_magic = CODA_MAGIC;
cfi->cfi_mapcount = 0;
cfi->cfi_container = host_file;
/* assume access intents are supported unless we hear otherwise */
--
2.30.2
On Thu, Nov 10, 2022 at 08:14:01PM -0500, Ahelenia Ziemiańska wrote:
> We have largely moved away from this approach, and we have better
> debugging tooling nowadays: kill it.
I still haven't received a response to my earlier email asking what
better debug tooling you are talking about.
I gave an example of an older bug, which has been fixed, where a Coda
inode accidentally ending up in ext4 through an mmap path. This ended up
scribbling over a spinlock in the coda_inode_info data which resulted
into pretty random system lockups.
This was actually quite hard to track down because there is no check
that `file_inode(vma->vm_file)` actually returns the inode of another
file system. So I'd be interesting what this tooling is or how it can
be used to better catch/identify such cases.
So I actually like this type of magic because it can potentially catch
some cases that should never happen. We could rename it to from MAGIC to
ASSERT, we don't remove asserts from code because of better debug tooling.
Those random 4 bytes in a Coda internal structure are not in a hot path.
Jan