[PATCH 0/3] scsi: sg: minor bugfix and cleanup

[PATCH 0/3] scsi: sg: minor bugfix and cleanup

[Yang Erkun]

Yang Erkun (3):
  scsi: sg: Fix sysctl sg-big-buff register during sg_init
  scsi: sg: Resolve soft lockup issue when opening /dev/sgX
  scsi: sg: Remove deprecated sg-big-buff

 drivers/scsi/sg.c | 88 ++++++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 50 deletions(-)

-- 
2.39.2

[PATCH 1/3] scsi: sg: Fix sysctl sg-big-buff register during sg_init

[Yang Erkun]

The sysctl interface sg-big-buff will no longer be available after this
commit. I believe this is a mistake, so fix it by invoking
register_sg_sysctls when everything is proceeding correctly.

Fixes: 26d1c80fd61e5 ("scsi/sg: move sg-big-buff sysctl to scsi/sg.c")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 drivers/scsi/sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 57fba34832ad..c93cc9323f56 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1689,13 +1689,13 @@ init_sg(void)
 	sg_sysfs_valid = 1;
 	rc = scsi_register_interface(&sg_interface);
 	if (0 == rc) {
+		register_sg_sysctls();
 #ifdef CONFIG_SCSI_PROC_FS
 		sg_proc_init();
 #endif				/* CONFIG_SCSI_PROC_FS */
 		return 0;
 	}
 	class_unregister(&sg_sysfs_class);
-	register_sg_sysctls();
 err_out:
 	unregister_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), SG_MAX_DEVS);
 	return rc;
-- 
2.39.2

[PATCH 2/3] scsi: sg: Resolve soft lockup issue when opening /dev/sgX

[Yang Erkun]

A soft lockup occurs after executing the following commands:

echo -1 > /sys/module/sg/parameters/def_reserved_size
exec 4<> /dev/sg0

watchdog: BUG: soft lockup - CPU#5 stuck for 26 seconds! [bash:537]
Modules loaded:
CPU: 5 UID: 0 PID: 537 Command: bash, kernel version 6.19.0-rc3+ #134,
PREEMPT disabled
Hardware: QEMU Standard PC (i440FX + PIIX, 1996), BIOS version
1.16.1-2.fc37 dated 04/01/2014
...
Call Trace:

  sg_build_reserve+0x5c/0xa0
  sg_add_sfp+0x168/0x270
  sg_open+0x16e/0x340
  chrdev_open+0xbe/0x230
  do_dentry_open+0x175/0x480
  vfs_open+0x34/0xf0
  do_open+0x265/0x3d0
  path_openat+0x110/0x290
  do_filp_open+0xc3/0x170
  do_sys_openat2+0x71/0xe0
  __x64_sys_openat+0x6d/0xa0
  do_syscall_64+0x62/0x310
  entry_SYSCALL_64_after_hwframe+0x76/0x7e

The solution is to use module_param_cb to validate and reject invalid
values for def_reserved_size.

Fixes: 6460e75a104d ("[SCSI] sg: fixes for large page_size")
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 drivers/scsi/sg.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c93cc9323f56..9e42618e20d5 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1621,10 +1621,35 @@ sg_remove_device(struct device *cl_dev)
 }
 
 module_param_named(scatter_elem_sz, scatter_elem_sz, int, S_IRUGO | S_IWUSR);
-module_param_named(def_reserved_size, def_reserved_size, int,
-		   S_IRUGO | S_IWUSR);
 module_param_named(allow_dio, sg_allow_dio, int, S_IRUGO | S_IWUSR);
 
+static int def_reserved_size_set(const char *val, const struct kernel_param *kp)
+{
+	int size, ret;
+
+	if (!val)
+		return -EINVAL;
+
+	ret = kstrtoint(val, 0, &size);
+	if (ret)
+		return ret;
+
+	/* limit "big buff" to 1 MB */
+	if (size < 0 || size > 1048576)
+		return -ERANGE;
+
+	def_reserved_size = size;
+	return 0;
+}
+
+static const struct kernel_param_ops def_reserved_size_ops = {
+	.set	= def_reserved_size_set,
+	.get	= param_get_int,
+};
+
+module_param_cb(def_reserved_size, &def_reserved_size_ops, &def_reserved_size,
+		   S_IRUGO | S_IWUSR);
+
 MODULE_AUTHOR("Douglas Gilbert");
 MODULE_DESCRIPTION("SCSI generic (sg) driver");
 MODULE_LICENSE("GPL");
-- 
2.39.2

[PATCH 3/3] scsi: sg: Remove deprecated sg-big-buff

[Yang Erkun]

These deprecated sysctl has been gone since commit 26d1c80fd61e5
("scsi/sg: move sg-big-buff sysctl to scsi/sg.c") and nobody has found
this. I believe it's time to remove them, which will allow us to clean
up a significant amount of code.

Signed-off-by: Yang Erkun <yangerkun@huawei.com>
---
 drivers/scsi/sg.c | 59 +++++++++--------------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 9e42618e20d5..b3bc95f6c2f7 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -81,14 +81,14 @@ static int sg_proc_init(void);
 
 #define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
 
-static int sg_big_buff = SG_DEF_RESERVED_SIZE;
 /* N.B. This variable is readable and writeable via
-   /proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
-   of this size (or less if there is not enough memory) will be reserved
-   for use by this file descriptor. [Deprecated usage: this variable is also
-   readable via /proc/sys/kernel/sg-big-buff if the sg driver is built into
-   the kernel (i.e. it is not a module).] */
-static int def_reserved_size = -1;	/* picks up init parameter */
+ * /proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
+ * of this size (or less if there is not enough memory) will be reserved
+ * for use by this file descriptor.
+ */
+
+/* picks up init parameter */
+static int def_reserved_size = SG_DEF_RESERVED_SIZE;
 static int sg_allow_dio = SG_ALLOW_DIO_DEF;
 
 static int scatter_elem_sz = SG_SCATTER_SZ;
@@ -1661,35 +1661,6 @@ MODULE_PARM_DESC(scatter_elem_sz, "scatter gather element "
 MODULE_PARM_DESC(def_reserved_size, "size of buffer reserved for each fd");
 MODULE_PARM_DESC(allow_dio, "allow direct I/O (default: 0 (disallow))");
 
-#ifdef CONFIG_SYSCTL
-#include <linux/sysctl.h>
-
-static const struct ctl_table sg_sysctls[] = {
-	{
-		.procname	= "sg-big-buff",
-		.data		= &sg_big_buff,
-		.maxlen		= sizeof(int),
-		.mode		= 0444,
-		.proc_handler	= proc_dointvec,
-	},
-};
-
-static struct ctl_table_header *hdr;
-static void register_sg_sysctls(void)
-{
-	if (!hdr)
-		hdr = register_sysctl("kernel", sg_sysctls);
-}
-
-static void unregister_sg_sysctls(void)
-{
-	unregister_sysctl_table(hdr);
-}
-#else
-#define register_sg_sysctls() do { } while (0)
-#define unregister_sg_sysctls() do { } while (0)
-#endif /* CONFIG_SYSCTL */
-
 static int __init
 init_sg(void)
 {
@@ -1699,10 +1670,6 @@ init_sg(void)
 		scatter_elem_sz = PAGE_SIZE;
 		scatter_elem_sz_prev = scatter_elem_sz;
 	}
-	if (def_reserved_size >= 0)
-		sg_big_buff = def_reserved_size;
-	else
-		def_reserved_size = sg_big_buff;
 
 	rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0), 
 				    SG_MAX_DEVS, "sg");
@@ -1714,7 +1681,6 @@ init_sg(void)
 	sg_sysfs_valid = 1;
 	rc = scsi_register_interface(&sg_interface);
 	if (0 == rc) {
-		register_sg_sysctls();
 #ifdef CONFIG_SCSI_PROC_FS
 		sg_proc_init();
 #endif				/* CONFIG_SCSI_PROC_FS */
@@ -1729,7 +1695,6 @@ init_sg(void)
 static void __exit
 exit_sg(void)
 {
-	unregister_sg_sysctls();
 #ifdef CONFIG_SCSI_PROC_FS
 	remove_proc_subtree("scsi/sg", NULL);
 #endif				/* CONFIG_SCSI_PROC_FS */
@@ -2205,10 +2170,8 @@ sg_add_sfp(Sg_device * sdp)
 	write_unlock_irqrestore(&sdp->sfd_lock, iflags);
 	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
 				      "sg_add_sfp: sfp=0x%p\n", sfp));
-	if (unlikely(sg_big_buff != def_reserved_size))
-		sg_big_buff = def_reserved_size;
 
-	bufflen = min_t(int, sg_big_buff,
+	bufflen = min_t(int, def_reserved_size,
 			max_sectors_bytes(sdp->device->request_queue));
 	sg_build_reserve(sfp, bufflen);
 	SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
@@ -2436,7 +2399,7 @@ sg_proc_write_adio(struct file *filp, const char __user *buffer,
 
 static int sg_proc_single_open_dressz(struct inode *inode, struct file *file)
 {
-	return single_open(file, sg_proc_seq_show_int, &sg_big_buff);
+	return single_open(file, sg_proc_seq_show_int, &def_reserved_size);
 }
 
 static ssize_t 
@@ -2453,7 +2416,7 @@ sg_proc_write_dressz(struct file *filp, const char __user *buffer,
 	if (err)
 		return err;
 	if (k <= 1048576) {	/* limit "big buff" to 1 MB */
-		sg_big_buff = k;
+		def_reserved_size = k;
 		return count;
 	}
 	return -ERANGE;
@@ -2626,7 +2589,7 @@ static int sg_proc_seq_show_debug(struct seq_file *s, void *v)
 
 	if (it && (0 == it->index))
 		seq_printf(s, "max_active_device=%d  def_reserved_size=%d\n",
-			   (int)it->max, sg_big_buff);
+			   (int)it->max, def_reserved_size);
 
 	read_lock_irqsave(&sg_index_lock, iflags);
 	sdp = it ? sg_lookup_dev(it->index) : NULL;
-- 
2.39.2

Re: [PATCH 1/3] scsi: sg: Fix sysctl sg-big-buff register during sg_init

[Bart Van Assche]

On 1/26/26 5:27 AM, Yang Erkun wrote:
> The sysctl interface sg-big-buff will no longer be available after this
> commit.

This patch restores the sg-big-buff ioctl functionality, isn't it? The
above sentence says something completely different. Please fix.

Thanks,

Bart.

Re: [PATCH 2/3] scsi: sg: Resolve soft lockup issue when opening /dev/sgX

[Bart Van Assche]

On 1/26/26 5:27 AM, Yang Erkun wrote:
> +	/* limit "big buff" to 1 MB */
> +	if (size < 0 || size > 1048576)
> +		return -ERANGE;

Why 1 MB? Please explain this in the patch description.

Thanks,

Bart.

Re: [PATCH 3/3] scsi: sg: Remove deprecated sg-big-buff

[Bart Van Assche]

On 1/26/26 5:27 AM, Yang Erkun wrote:
> These deprecated sysctl has been gone since commit 26d1c80fd61e5

gone -> broken

> ("scsi/sg: move sg-big-buff sysctl to scsi/sg.c") and nobody has found
> this. I believe it's time to remove them, which will allow us to clean

them -> it

> up a significant amount of code.

Thanks,

Bart.

Re: [PATCH 2/3] scsi: sg: Resolve soft lockup issue when opening /dev/sgX

[yangerkun]

在 2026/1/27 1:04, Bart Van Assche 写道:
> On 1/26/26 5:27 AM, Yang Erkun wrote:
>> +    /* limit "big buff" to 1 MB */
>> +    if (size < 0 || size > 1048576)
>> +        return -ERANGE;
> 
> Why 1 MB? Please explain this in the patch description.

Hi,

Thank you very much for your review! I obtained this limit from the
function sg_proc_write_dressz, which states to "limit 'big buff' to 1
MB." Additionally, https://tldp.org/HOWTO/SCSI-Generic-HOWTO/proc.html
also mentions that "values between 0 and 1,048,576 (which is 2 ** 20)
are accepted."

Thanks again for your feedback! All your suggestions will be update in
the next version.

Thanks,
Erkun.

> 
> Thanks,
> 
> Bart.