[PATCH v2 00/15] magic-number.rst funeral rites

[PATCH v2 00/15] magic-number.rst funeral rites

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 4514 bytes --]

This is a follow-up for the 18+1-patch series (
https://lore.kernel.org/linux-kernel/8389a7b85b5c660c6891b1740b5dacc53491a41b.1663280877.git.nabijaczleweli@nabijaczleweli.xyz/
https://lore.kernel.org/linux-kernel/20220927003727.slf4ofb7dgum6apt@tarta.nabijaczleweli.xyz/
) I sent in September, and the same reasoning applies:

The entire file blames back to the start of git
(minus whitespace from the RST translation and a typo fix):
  * there are changelog comments for March 1994 through to Linux 2.5.74
  * struct tty_ldisc is two pointers nowadays, so naturally no magic
  * GDA_MAGIC is defined but unused, and it's been this way
    since start-of-git
  * M3_CARD_MAGIC isn't defined, because
    commit d56b9b9c464a ("[PATCH] The scheduled removal of some OSS
    drivers") removed the entire driver in 2006
  * CS_CARD_MAGIC likewise since
    commit b5d425c97f7d ("more scheduled OSS driver removal") in 2007
  * KMALLOC_MAGIC and VMALLOC_MAGIC were removed in
    commit e38e0cfa48ac ("[ALSA] Remove kmalloc wrappers"),
    six months after start of git
  * SLAB_C_MAGIC has never even appeared in git
    (removed in 2.4.0-test3pre6)
  * &c., &c., &c.

magic-number.rst is a low-value historial relic at best and misleading
cruft at worst.

This latter half cleans out the remaining entries (either by recognising
that they aren't actually magic numbers or by cutting them out entirely)
and inters the file.

amd64 allyesconfig builds; this largely touches code that would be
exceedingly expensive to test (and largely untouched since the git
import), but is very receptive to static analysis.

v2:
  Messages restyled
  Moved printk() in synclink_cs.c became pr_warn
  (__func__ instead of prescribed hard function name per checkpatch.pl)

Ahelenia Ziemiańska (15):
  hamradio: baycom: remove BAYCOM_MAGIC
  hamradio: yam: remove YAM_MAGIC
  pcmcia: synclink_cs: remove MGSLPC_MAGIC
  pcmcia: synclink_cs: remove dead paranoia_check, warn for missing line
  coda: remove CODA_MAGIC
  Documentation: remove PG_MAGIC (not a magic number)
  MIPS: IP27: clean out sn/nmi.h
  MIPS: IP27: remove KV_MAGIC
  x86/APM: remove APM_BIOS_MAGIC
  scsi: acorn: remove QUEUE_MAGIC_{FREE,USED}
  hdlcdrv: remove HDLCDRV_MAGIC
  drivers: net: slip: remove SLIP_MAGIC
  fcntl: remove FASYNC_MAGIC
  scsi: ncr53c8xx: replace CCB_MAGIC with bool busy
  Documentation: remove magic-number.rst

 Documentation/process/index.rst               |  1 -
 Documentation/process/magic-number.rst        | 85 -----------------
 .../translations/it_IT/process/index.rst      |  1 -
 .../it_IT/process/magic-number.rst            | 91 -------------------
 .../translations/zh_CN/process/index.rst      |  1 -
 .../zh_CN/process/magic-number.rst            | 74 ---------------
 .../translations/zh_TW/process/index.rst      |  1 -
 .../zh_TW/process/magic-number.rst            | 77 ----------------
 arch/mips/include/asm/sn/klkernvars.h         |  8 +-
 arch/mips/include/asm/sn/nmi.h                | 60 ------------
 arch/mips/sgi-ip27/ip27-klnuma.c              |  1 -
 arch/x86/kernel/apm_32.c                      |  9 +-
 drivers/char/pcmcia/synclink_cs.c             | 79 +---------------
 drivers/net/hamradio/baycom_epp.c             | 15 +--
 drivers/net/hamradio/baycom_par.c             |  1 -
 drivers/net/hamradio/baycom_ser_fdx.c         |  3 +-
 drivers/net/hamradio/baycom_ser_hdx.c         |  3 +-
 drivers/net/hamradio/hdlcdrv.c                |  9 +-
 drivers/net/hamradio/yam.c                    |  8 +-
 drivers/net/slip/slip.c                       | 11 +--
 drivers/net/slip/slip.h                       |  4 -
 drivers/scsi/arm/queue.c                      | 21 -----
 drivers/scsi/ncr53c8xx.c                      | 25 ++---
 fs/coda/cnode.c                               |  2 +-
 fs/coda/coda_fs_i.h                           |  2 -
 fs/coda/file.c                                |  1 -
 fs/fcntl.c                                    |  6 --
 include/linux/fs.h                            |  3 -
 include/linux/hdlcdrv.h                       |  2 -
 29 files changed, 29 insertions(+), 575 deletions(-)
 delete mode 100644 Documentation/process/magic-number.rst
 delete mode 100644 Documentation/translations/it_IT/process/magic-number.rst
 delete mode 100644 Documentation/translations/zh_CN/process/magic-number.rst
 delete mode 100644 Documentation/translations/zh_TW/process/magic-number.rst

-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 10/15] scsi: acorn: remove QUEUE_MAGIC_{FREE,USED}

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 6033 bytes --]

We have largely moved away from this approach, and we have better
debugging instrumentation nowadays: kill it.

Link: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 Documentation/process/magic-number.rst        |  2 --
 .../it_IT/process/magic-number.rst            |  2 --
 .../zh_CN/process/magic-number.rst            |  2 --
 .../zh_TW/process/magic-number.rst            |  2 --
 drivers/scsi/arm/queue.c                      | 21 -------------------
 5 files changed, 29 deletions(-)

diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
index 77a96a79c7e8..c1c68c713cbc 100644
--- a/Documentation/process/magic-number.rst
+++ b/Documentation/process/magic-number.rst
@@ -72,6 +72,4 @@ FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.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 ef509265a3df..5b609ca78a14 100644
--- a/Documentation/translations/it_IT/process/magic-number.rst
+++ b/Documentation/translations/it_IT/process/magic-number.rst
@@ -78,6 +78,4 @@ FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.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 6437c408dd67..ab4d4e32b61f 100644
--- a/Documentation/translations/zh_CN/process/magic-number.rst
+++ b/Documentation/translations/zh_CN/process/magic-number.rst
@@ -61,6 +61,4 @@ FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.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 453cc205e6c3..a6131d978189 100644
--- a/Documentation/translations/zh_TW/process/magic-number.rst
+++ b/Documentation/translations/zh_TW/process/magic-number.rst
@@ -64,6 +64,4 @@ FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.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/drivers/scsi/arm/queue.c b/drivers/scsi/arm/queue.c
index 978df23ce188..fe7d814a73a9 100644
--- a/drivers/scsi/arm/queue.c
+++ b/drivers/scsi/arm/queue.c
@@ -26,27 +26,11 @@
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_tcq.h>
 
-#define DEBUG
-
 typedef struct queue_entry {
 	struct list_head   list;
 	struct scsi_cmnd   *SCpnt;
-#ifdef DEBUG
-	unsigned long	   magic;
-#endif
 } QE_t;
 
-#ifdef DEBUG
-#define QUEUE_MAGIC_FREE	0xf7e1c9a3
-#define QUEUE_MAGIC_USED	0xf7e1cc33
-
-#define SET_MAGIC(q,m)	((q)->magic = (m))
-#define BAD_MAGIC(q,m)	((q)->magic != (m))
-#else
-#define SET_MAGIC(q,m)	do { } while (0)
-#define BAD_MAGIC(q,m)	(0)
-#endif
-
 #include "queue.h"
 
 #define NR_QE	32
@@ -74,7 +58,6 @@ int queue_initialise (Queue_t *queue)
 	queue->alloc = q = kmalloc_array(nqueues, sizeof(QE_t), GFP_KERNEL);
 	if (q) {
 		for (; nqueues; q++, nqueues--) {
-			SET_MAGIC(q, QUEUE_MAGIC_FREE);
 			q->SCpnt = NULL;
 			list_add(&q->list, &queue->free);
 		}
@@ -119,9 +102,7 @@ int __queue_add(Queue_t *queue, struct scsi_cmnd *SCpnt, int head)
 	list_del(l);
 
 	q = list_entry(l, QE_t, list);
-	BUG_ON(BAD_MAGIC(q, QUEUE_MAGIC_FREE));
 
-	SET_MAGIC(q, QUEUE_MAGIC_USED);
 	q->SCpnt = SCpnt;
 
 	if (head)
@@ -144,9 +125,7 @@ static struct scsi_cmnd *__queue_remove(Queue_t *queue, struct list_head *ent)
 	 */
 	list_del(ent);
 	q = list_entry(ent, QE_t, list);
-	BUG_ON(BAD_MAGIC(q, QUEUE_MAGIC_USED));
 
-	SET_MAGIC(q, QUEUE_MAGIC_FREE);
 	list_add(ent, &queue->free);
 
 	return q->SCpnt;
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 14/15] scsi: ncr53c8xx: replace CCB_MAGIC with bool busy

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 6405 bytes --]

The only non-boolean check might as well be, since it just early-exits
instead of noting the bug: lower it to a boolean and make it less
confusing.

As for magic numbers, we have largely moved away from this approach,
and we have better debugging instrumentation nowadays: kill it.

Link: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 Documentation/process/magic-number.rst        |  1 -
 .../it_IT/process/magic-number.rst            |  1 -
 .../zh_CN/process/magic-number.rst            |  1 -
 .../zh_TW/process/magic-number.rst            |  1 -
 drivers/scsi/ncr53c8xx.c                      | 25 ++++++-------------
 5 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
index 6e432917a5a8..5a8c2755ac9c 100644
--- a/Documentation/process/magic-number.rst
+++ b/Documentation/process/magic-number.rst
@@ -68,5 +68,4 @@ Changelog::
 ===================== ================ ======================== ==========================================
 Magic Name            Number           Structure                File
 ===================== ================ ======================== ==========================================
-CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
 ===================== ================ ======================== ==========================================
diff --git a/Documentation/translations/it_IT/process/magic-number.rst b/Documentation/translations/it_IT/process/magic-number.rst
index 7d4c117ac626..2fbc1876534a 100644
--- a/Documentation/translations/it_IT/process/magic-number.rst
+++ b/Documentation/translations/it_IT/process/magic-number.rst
@@ -74,5 +74,4 @@ Registro dei cambiamenti::
 ===================== ================ ======================== ==========================================
 Nome magico           Numero           Struttura                File
 ===================== ================ ======================== ==========================================
-CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
 ===================== ================ ======================== ==========================================
diff --git a/Documentation/translations/zh_CN/process/magic-number.rst b/Documentation/translations/zh_CN/process/magic-number.rst
index c17e3f20440a..f8ec4767bc4e 100644
--- a/Documentation/translations/zh_CN/process/magic-number.rst
+++ b/Documentation/translations/zh_CN/process/magic-number.rst
@@ -57,5 +57,4 @@ Linux 魔术数
 ===================== ================ ======================== ==========================================
 魔术数名              数字             结构                     文件
 ===================== ================ ======================== ==========================================
-CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
 ===================== ================ ======================== ==========================================
diff --git a/Documentation/translations/zh_TW/process/magic-number.rst b/Documentation/translations/zh_TW/process/magic-number.rst
index e2eeb74e7192..0ccc60bee3d6 100644
--- a/Documentation/translations/zh_TW/process/magic-number.rst
+++ b/Documentation/translations/zh_TW/process/magic-number.rst
@@ -60,5 +60,4 @@ Linux 魔術數
 ===================== ================ ======================== ==========================================
 魔術數名              數字             結構                     文件
 ===================== ================ ======================== ==========================================
-CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
 ===================== ================ ======================== ==========================================
diff --git a/drivers/scsi/ncr53c8xx.c b/drivers/scsi/ncr53c8xx.c
index 4458449c960b..928417fca495 100644
--- a/drivers/scsi/ncr53c8xx.c
+++ b/drivers/scsi/ncr53c8xx.c
@@ -1095,15 +1095,6 @@ typedef u32 tagmap_t;
 #define NS_WIDE		(2)
 #define NS_PPR		(4)
 
-/*==========================================================
-**
-**	Misc.
-**
-**==========================================================
-*/
-
-#define CCB_MAGIC	(0xf2691ad2)
-
 /*==========================================================
 **
 **	Declaration of structs.
@@ -1567,7 +1558,7 @@ struct ccb {
 	struct ccb *	link_ccb;	/* Host adapter CCB chain	*/
 	struct list_head link_ccbq;	/* Link to unit CCB queue	*/
 	u32		startp;		/* Initial data pointer		*/
-	u_long		magic;		/* Free / busy  CCB flag	*/
+	bool		busy;
 };
 
 #define CCB_PHYS(cp,lbl)	(cp->p_ccb + offsetof(struct ccb, lbl))
@@ -4356,7 +4347,7 @@ static int ncr_queue_command (struct ncb *np, struct scsi_cmnd *cmd)
 	*/
 
 	/* activate this job.  */
-	cp->magic		= CCB_MAGIC;
+	cp->busy		= true;
 
 	/*
 	**	insert next CCBs into start queue.
@@ -4667,7 +4658,7 @@ void ncr_complete (struct ncb *np, struct ccb *cp)
 	**	Sanity check
 	*/
 
-	if (!cp || cp->magic != CCB_MAGIC || !cp->cmd)
+	if (!cp || !cp->busy || !cp->cmd)
 		return;
 
 	/*
@@ -6998,7 +6989,7 @@ static struct ccb *ncr_get_ccb(struct ncb *np, struct scsi_cmnd *cmd)
 		qp = ncr_list_pop(&lp->free_ccbq);
 		if (qp) {
 			cp = list_entry(qp, struct ccb, link_ccbq);
-			if (cp->magic) {
+			if (cp->busy) {
 				PRINT_ADDR(cmd, "ccb free list corrupted "
 						"(@%p)\n", cp);
 				cp = NULL;
@@ -7030,17 +7021,17 @@ static struct ccb *ncr_get_ccb(struct ncb *np, struct scsi_cmnd *cmd)
 	**	Wait until available.
 	*/
 #if 0
-	while (cp->magic) {
+	while (cp->busy) {
 		if (flags & SCSI_NOSLEEP) break;
 		if (tsleep ((caddr_t)cp, PRIBIO|PCATCH, "ncr", 0))
 			break;
 	}
 #endif
 
-	if (cp->magic)
+	if (cp->busy)
 		return NULL;
 
-	cp->magic = 1;
+	cp->busy = true;
 
 	/*
 	**	Move to next available tag if tag used.
@@ -7119,7 +7110,7 @@ static void ncr_free_ccb (struct ncb *np, struct ccb *cp)
 		}
 	}
 	cp -> host_status = HS_IDLE;
-	cp -> magic = 0;
+	cp -> busy = false;
 	if (cp->queued) {
 		--np->queuedccbs;
 		cp->queued = 0;
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 00/15] magic-number.rst funeral rites

[Jakub Kicinski]

On Wed, 2 Nov 2022 00:04:54 +0100 Ahelenia Ziemiańska wrote:
> This is a follow-up for the 18+1-patch series (
> https://lore.kernel.org/linux-kernel/8389a7b85b5c660c6891b1740b5dacc53491a41b.1663280877.git.nabijaczleweli@nabijaczleweli.xyz/
> https://lore.kernel.org/linux-kernel/20220927003727.slf4ofb7dgum6apt@tarta.nabijaczleweli.xyz/
> ) I sent in September, and the same reasoning applies:

No idea how you want this to get merged, but FWIW you can add my

Acked-by: Jakub Kicinski <kuba@kernel.org>

to patches 1, 2, 11 and 12.

Re: [PATCH v2 14/15] scsi: ncr53c8xx: replace CCB_MAGIC with bool busy

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

On Wed, 2022-11-02 at 00:06 +0100, Ahelenia Ziemiańska wrote:
[...]
> @@ -4356,7 +4347,7 @@ static int ncr_queue_command (struct ncb *np,
> struct scsi_cmnd *cmd)
>  	*/
>  
>  	/* activate this job.  */
> -	cp->magic		= CCB_MAGIC;
> +	cp->busy		= true;
>  
>  	/*
>  	**	insert next CCBs into start queue.

Are you sure this is just an internal magic number?  The way these old
scripts engines used to work is that the CCB array forms a mailbox and
the card continually scans the mailbox to see if it has any work to do.
These magic values are often a signal to the card that the CCB array
contains a valid entry it hasn't seen and it should work on it, so the
card might be expecting to see this full 32 bit value.

Regards,

James


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 14/15] scsi: ncr53c8xx: replace CCB_MAGIC with bool busy

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On Fri, Nov 04, 2022 at 03:57:20PM -0400, James Bottomley wrote:
> On Wed, 2022-11-02 at 00:06 +0100, Ahelenia Ziemiańska wrote:
> [...]
> > @@ -4356,7 +4347,7 @@ static int ncr_queue_command (struct ncb *np,
> > struct scsi_cmnd *cmd)
> >  	*/
> >  
> >  	/* activate this job.  */
> > -	cp->magic		= CCB_MAGIC;
> > +	cp->busy		= true;
> >  
> >  	/*
> >  	**	insert next CCBs into start queue.
> 
> Are you sure this is just an internal magic number?  The way these old
> scripts engines used to work is that the CCB array forms a mailbox and
> the card continually scans the mailbox to see if it has any work to do.
> These magic values are often a signal to the card that the CCB array
> contains a valid entry it hasn't seen and it should work on it, so the
> card might be expecting to see this full 32 bit value.

AFAICT this is an internal flag: in FreeBSD, whence this is copied, this
driver is introduced in 37bd2c9c33d9684c0bf650e02579ebb329724d6a[1],
and AFAICT it's used in exactly the same way as it is here; the manual[2]
doesn't mention it at all; neither do the scripts; and the layout is
vastly different between there and current Linux; so I'm happy to
conclude it's not used by the device, I think?

1: https://cgit.freebsd.org/src/commit/?id=37bd2c9c33d9684c0bf650e02579ebb329724d6a
2: http://bitsavers.trailing-edge.com/components/ncr_symbios/scsi/53C810/53C810_PCI-SCSI_IO_Processor_Data_Manual_May1993.pdf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]