[05/15] coda: remove CODA_MAGIC
Commit Message
We have largely moved away from this approach,
and we have better debugging tooling nowadays: kill it
Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
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(-)
Comments
On 10/27/22 05:42, наб wrote:
> We have largely moved away from this approach,
> and we have better debugging tooling nowadays: kill it
>
Again, see [1] and [2].
[1]: https://lore.kernel.org/linux-doc/47c2bffb-6bfe-7f5d-0d2d-3cbb99d31019@gmail.com/
[2]: https://lore.kernel.org/linux-doc/9d96c96d-dfc7-7749-07d4-2601c00661c2@gmail.com/
On Fri, Oct 28, 2022 at 08:23:38PM +0700, Bagas Sanjaya wrote:
> On 10/27/22 05:42, наб wrote:
> > We have largely moved away from this approach,
> > and we have better debugging tooling nowadays: kill it
> >
>
> Again, see [1] and [2].
>
> [1]: https://lore.kernel.org/linux-doc/47c2bffb-6bfe-7f5d-0d2d-3cbb99d31019@gmail.com/
> [2]: https://lore.kernel.org/linux-doc/9d96c96d-dfc7-7749-07d4-2601c00661c2@gmail.com/
I guess I must be old-school because I appreciate magic numbers and
assertions to catch situations that should obviously never happen.
This https://lkml.org/lkml/2019/4/2/923 bug would have been caught a lot
sooner if ext4 had happened to check a magic number in vma->vm_file. As
it was, we ended up passing a Coda file instead of a file handle for the
the underlying filesystem which then ended up with the wrong inode and
their mmap_sem happened to scribble the spinlock on the Coda inode.
This wasn't really ext4's fault, it wasn't expecting to get a mmapped
region with another file system's file handle, but that is exactly the
point of these magic numbers, to catch the unexpected. Not sure what
better debug tooling would catch that unless you were already expecting
the problem.
Jan
@@ -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``
@@ -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``
@@ -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``
@@ -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``
@@ -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;
@@ -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 */
@@ -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 */