- kernelnewbies@kernelnewbies.org - Kernelnewbies Mailing Lists

Re: [PATCH] drivers: pnp: proc.c: Handle errors while attaching devices
by Leon Romanovsky April 29, 2021

April 29, 2021

On Mon, Apr 26, 2021 at 11:20:32PM +0530, bkkarthik wrote: > On 21/04/26 08:04AM, Leon Romanovsky wrote: > > On Sun, Apr 25, 2021 at 01:13:01AM +0530, Anupama K Patil wrote: > > > isapnp_proc_init() does not look at the return value from > > > isapnp_proc_attach_device(). Check for this return value in > > > isapnp_proc_detach_device(). > > > > > > Cleanup in isapnp_proc_detach_device and > > > isapnp_proc_detach_bus() for cleanup. > > > > > > Changed sprintf() to the kernel-space function scnprintf() as it returns > > > the actual number of bytes written. > > > > > > Removed unnecessary variables de, e of type 'struct proc_dir_entry' to > > > save memory. <...> > > > +static int isapnp_proc_detach_device(struct pnp_dev *dev) > > > +{ > > > + proc_remove(dev->procent); > > > + dev->procent = NULL; > > > + return 0; > > > +} > > > + > > > +static int isapnp_proc_detach_bus(struct pnp_card *bus) > > > +{ > > > + proc_remove(bus->procdir); > > > + return 0; > > > +} > > > > Please don't add one line functions that are called only once and have > > return value that no one care about it. > > These were only intended for a clean-up job, the idea of this function came from how PCI handles procfs. > Maybe those should be changed? Probably, the CONFIG_PROC_FS around pci_proc_*() is not needed too. Thanks

2 2

Problem building an out of tree modules for kernel version 5.8.0
by Michael McCann April 28, 2021

April 28, 2021

I'm trying to build out of tree modules for kernel version 5.8.0 for a X86 host. Below are the results when I attempt to compile the modules. *test@test-GA-78LMT-USB3-6-0:~/X86module$ ./BuildDrivermake: Entering directory '/home/test/LinuxKernel/linux-git' CC [M] /home/test/X86module/thread.oIn file included from ./include/linux/types.h:6, from ./include/linux/limits.h:6, from ./include/linux/kernel.h:7, from /home/test/X86module/thread.c:3:./include/uapi/linux/types.h:5:10: fatal error: asm/types.h: No such file or directory 5 | #include <asm/types.h> | ^~~~~~~~~~~~~compilation terminated.make[1]: *** [scripts/Makefile.build:281: /home/test/X86module/thread.o] Error 1make: *** [Makefile:1756: /home/test/X86module] Error 2make: Leaving directory '/home/test/LinuxKernel/linux-git'* This is the source code for the module being compiled: */* out-of-tree kthreads module */#include <linux/kernel.h>#include <linux/module.h>#include <linux/kthread.h>#include <linux/delay.h>#define LOGLEVEL KERN_INFOstatic struct task_struct *mythread;static int work(void *dummy);static int __init kthread_init(void) { int r; printk(LOGLEVEL "kthread module start\n"); mythread = kthread_create(work, NULL, "mykthread"); if (mythread > 0) { printk(LOGLEVEL "kthread created\n"); r = wake_up_process(mythread); if (r) { printk(LOGLEVEL "kthread started\n"); printk(LOGLEVEL "kthread pid = %0#10x %d\n", mythread->pid, mythread->pid); } else printk(LOGLEVEL "kthread already running??\n"); } else printk(LOGLEVEL "kthread_create failed\n"); return(0);}static int work(void *dummy) { int i = 1; while (1) { ssleep(1); printk(LOGLEVEL "mykthread i = %d\n",i); i++; if (kthread_should_stop()) { printk(LOGLEVEL "my kthread is stopping...\n"); do_exit(0); } }}static void __exit kthread_exit(void) { int r; r = kthread_stop(mythread); printk(KERN_INFO "kthread module exit\n");}module_init(kthread_init);module_exit(kthread_exit);MODULE_LICENSE("GPL");MODULE_AUTHOR("Michael McCann");MODULE_DESCRIPTION("kernal threads experiment");*

1 0

Re: [PATCH] MAINTAINERS: mark isapnp as obsolete
by Leon Romanovsky April 28, 2021

April 28, 2021

On Wed, Apr 28, 2021 at 02:17:00PM +0200, Jaroslav Kysela wrote: > Dne 28. 04. 21 v 13:11 Greg KH napsal(a): > > On Wed, Apr 28, 2021 at 04:35:36PM +0530, Anupama K Patil wrote: > >> isapnp code is very old and according to this link > >> https://en.wikipedia.org/wiki/Legacy_Plug_and_Play#Specifications > >> from Wikipedia, even Windows Vista disabled ISA PnP by default. > >> > >> This change is in follow up to > >> https://lore.kernel.org/lkml/20210422180322.7wlyg63kv3n2k6id@ubuntu/T/#u > >> and https://lore.kernel.org/lkml/20210424194301.jmsqpycvsm7izbk3@ubuntu/T/#u > >> > >> Suggested-by: B K Karthik <bkkarthik(a)pesu.pes.edu> > >> Signed-off-by: Anupama K Patil <anupamakpatil123(a)gmail.com> > > > > Has the maintainer said this is unsupported? > > I've no idea, if there're any users at the time, but there are many drivers > which depend on this code. IMHO, this is exactly "Odd fixes" - code not really dead, but not really alive too. Thanks > > I'll try to reply to patch threads. > > Jaroslav > > -- > Jaroslav Kysela <perex(a)perex.cz> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

3 2

Re: [PATCH] drivers: pnp: proc.c: Handle errors while attaching devices
by Leon Romanovsky April 28, 2021

April 28, 2021

On Wed, Apr 28, 2021 at 02:04:49PM +0200, Jaroslav Kysela wrote: > Dne 26. 04. 21 v 19:50 bkkarthik napsal(a): > > On 21/04/26 08:04AM, Leon Romanovsky wrote: > >> On Sun, Apr 25, 2021 at 01:13:01AM +0530, Anupama K Patil wrote: > >>> isapnp_proc_init() does not look at the return value from > >>> isapnp_proc_attach_device(). Check for this return value in > >>> isapnp_proc_detach_device(). > >>> > >>> Cleanup in isapnp_proc_detach_device and > >>> isapnp_proc_detach_bus() for cleanup. > >>> > >>> Changed sprintf() to the kernel-space function scnprintf() as it returns > >>> the actual number of bytes written. > >>> > >>> Removed unnecessary variables de, e of type 'struct proc_dir_entry' to > >>> save memory. > >> > >> What exactly do you fix for such an old code? > > > > I was not aware that this code is so old. This fix was made after checkpatch reported assignment inside an if-statement. > > Please ignore this patch if th change is not necessary as the code is probably not being used anywhere :) > > > > Maybe the code has to be marked as obsolete in the MAINTAINERS file to prevent patches being sent? > > > >> > >>> > >>> Suggested-by: Shuah Khan <skhan(a)linuxfoundation.org> > >>> Co-developed-by: B K Karthik <bkkarthik(a)pesu.pes.edu> > >>> Signed-off-by: B K Karthik <bkkarthik(a)pesu.pes.edu> > >>> Signed-off-by: Anupama K Patil <anupamakpatil123(a)gmail.com> > >>> --- > >>> drivers/pnp/isapnp/proc.c | 40 +++++++++++++++++++++++++++++---------- > >>> 1 file changed, 30 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c > >>> index 785a796430fa..46ebc24175b7 100644 > >>> --- a/drivers/pnp/isapnp/proc.c > >>> +++ b/drivers/pnp/isapnp/proc.c > >>> @@ -54,34 +54,54 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { > >>> .proc_read = isapnp_proc_bus_read, > >>> }; > >>> > >>> +static int isapnp_proc_detach_device(struct pnp_dev *dev) > >>> +{ > >>> + proc_remove(dev->procent); > >>> + dev->procent = NULL; > >>> + return 0; > >>> +} > >>> + > >>> +static int isapnp_proc_detach_bus(struct pnp_card *bus) > >>> +{ > >>> + proc_remove(bus->procdir); > >>> + return 0; > >>> +} > >> > >> Please don't add one line functions that are called only once and have > >> return value that no one care about it. > > > > These were only intended for a clean-up job, the idea of this function came from how PCI handles procfs. > > Maybe those should be changed? > > Which code you refer? I see: > > for_each_pci_dev(dev) > pci_proc_attach_device(dev); He talks about isapnp_proc_detach_*() functions. > > > The error codes are ignored, too. It does not harm, if proc entries are not > created (in this case - the system is unstable anyway). We should concentrate > only to the wrong pointers usage. > > Jaroslav > > -- > Jaroslav Kysela <perex(a)perex.cz> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

3 3

[PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
by Anupama K Patil April 28, 2021

April 28, 2021

de, e are two variables of the type 'struct proc_dir_entry' which can be removed to save memory. This also fixes a coding style issue reported by checkpatch where we are suggested to make assignment outside the if statement. Signed-off-by: Anupama K Patil <anupamakpatil123(a)gmail.com> --- drivers/pnp/isapnp/proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..1ae458c02656 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -57,21 +57,20 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { static int isapnp_proc_attach_device(struct pnp_dev *dev) { struct pnp_card *bus = dev->card; - struct proc_dir_entry *de, *e; char name[16]; - if (!(de = bus->procdir)) { + if (!bus->procdir) { sprintf(name, "%02x", bus->number); - de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); + if (!bus->procdir) return -ENOMEM; } sprintf(name, "%02x", dev->number); - e = dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, de, + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) return -ENOMEM; - proc_set_size(e, 256); + proc_set_size(dev->procent, 256); return 0; } -- 2.25.1

5 5

[PATCH] drivers: pnp: proc.c: Handle errors while attaching devices
by Anupama K Patil April 28, 2021

April 28, 2021

isapnp_proc_init() does not look at the return value from isapnp_proc_attach_device(). Check for this return value in isapnp_proc_detach_device(). Cleanup in isapnp_proc_detach_device and isapnp_proc_detach_bus() for cleanup. Changed sprintf() to the kernel-space function scnprintf() as it returns the actual number of bytes written. Removed unnecessary variables de, e of type 'struct proc_dir_entry' to save memory. Suggested-by: Shuah Khan <skhan(a)linuxfoundation.org> Co-developed-by: B K Karthik <bkkarthik(a)pesu.pes.edu> Signed-off-by: B K Karthik <bkkarthik(a)pesu.pes.edu> Signed-off-by: Anupama K Patil <anupamakpatil123(a)gmail.com> --- drivers/pnp/isapnp/proc.c | 40 +++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/pnp/isapnp/proc.c b/drivers/pnp/isapnp/proc.c index 785a796430fa..46ebc24175b7 100644 --- a/drivers/pnp/isapnp/proc.c +++ b/drivers/pnp/isapnp/proc.c @@ -54,34 +54,54 @@ static const struct proc_ops isapnp_proc_bus_proc_ops = { .proc_read = isapnp_proc_bus_read, }; +static int isapnp_proc_detach_device(struct pnp_dev *dev) +{ + proc_remove(dev->procent); + dev->procent = NULL; + return 0; +} + +static int isapnp_proc_detach_bus(struct pnp_card *bus) +{ + proc_remove(bus->procdir); + return 0; +} + static int isapnp_proc_attach_device(struct pnp_dev *dev) { struct pnp_card *bus = dev->card; - struct proc_dir_entry *de, *e; char name[16]; - if (!(de = bus->procdir)) { - sprintf(name, "%02x", bus->number); - de = bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); - if (!de) + if (!bus->procdir) { + scnprintf(name, 16, "%02x", bus->number); + bus->procdir = proc_mkdir(name, isapnp_proc_bus_dir); + if (!bus->procdir) return -ENOMEM; } - sprintf(name, "%02x", dev->number); - e = dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, de, + scnprintf(name, 16, "%02x", dev->number); + dev->procent = proc_create_data(name, S_IFREG | S_IRUGO, bus->procdir, &isapnp_proc_bus_proc_ops, dev); - if (!e) + if (!dev->procent) { + isapnp_proc_detach_bus(bus); return -ENOMEM; - proc_set_size(e, 256); + } + proc_set_size(dev->procent, 256); return 0; } int __init isapnp_proc_init(void) { struct pnp_dev *dev; + int dev_attach; isapnp_proc_bus_dir = proc_mkdir("bus/isapnp", NULL); protocol_for_each_dev(&isapnp_protocol, dev) { - isapnp_proc_attach_device(dev); + dev_attach = isapnp_proc_attach_device(dev); + if (!dev_attach) { + pr_info("procfs: pnp: Unable to attach the device, not enough memory"); + isapnp_proc_detach_device(dev); + return -ENOMEM; + } } return 0; } -- 2.25.1

5 4

Re: [kernel]:overlayfs: invalid origin (root/bmcweb_persistent_data.json, ftype=8000, origin ftype=4000)
by Amir Goldstein April 27, 2021

April 27, 2021

On Thu, Apr 15, 2021 at 2:03 PM www <ouyangxuan10(a)163.com> wrote: > > Dear Amir, kevin, miklos, > > An open source project uses overlay filesystem and systemd. when the system first time boot, there is an error in the file system: > kernel: overlayfs: invalid origin (root/bmcweb_persistent_data.json, ftype=8000, origin ftype=4000) > > What's the reason for this? Can you give some help and suggestions to solve this problem? > > The details are as follows: > 1. linux kernel version: > root@N/A:~# uname -a Linux N/A 5.1.5-yocto-s-dirty-1605363 #1 Tue Apr 13 01:13:48 UTC 2021 armv6l GNU/Linux > > 2. mount command > mount -t overlay -o lowerdir=$rodir,upperdir=$upper,workdir=$work cow /root > (rodir=run/initramfs/ro, upper=run/initramfs/rw/cow, work=rwdir=run/initramfs/rw/work) > > 3. mountinfo > root@N/A:~# cat /proc/self/mountinfo > 13 21 0:6 / /dev rw,relatime shared:8 - devtmpfs dev rw,size=220800k,nr_inodes=55200,mode=755 > 14 21 0:13 / /sys rw,relatime shared:3 - sysfs sys rw > 15 21 0:4 / /proc rw,relatime shared:2 - proc proc rw > 16 21 0:14 / /run rw,nosuid,nodev shared:11 - tmpfs tmpfs rw,mode=755 > 17 16 7:0 / /run/initramfs/ro ro,relatime shared:12 - squashfs /dev/loop0 ro > 18 16 31:5 / /run/initramfs/rw rw,relatime shared:13 - jffs2 /dev/mtdblock5 rw > 21 0 0:15 / / rw,relatime shared:1 - overlay cow rw,lowerdir=run/initramfs/ro,upperdir=run/initramfs/rw/cow,workdir=run/initramfs/rw/work > 22 13 0:17 / /dev/shm rw,nosuid,nodev shared:9 - tmpfs tmpfs rw > 23 13 0:18 / /dev/pts rw,relatime shared:10 - devpts devpts rw,gid=5,mode=620,ptmxmode=000 > 24 14 0:19 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:4 - tmpfs tmpfs ro,mode=755 > 25 24 0:20 / /sys/fs/cgroup/unified rw,nosuid,nodev,noexec,relatime shared:5 - cgroup2 cgroup2 rw,nsdelegate > 26 24 0:21 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:6 - cgroup cgroup rw,xattr,name=systemd > 27 14 0:22 / /sys/fs/bpf rw,nosuid,nodev,noexec,relatime shared:7 - bpf bpf rw,mode=700 > 28 14 0:7 / /sys/kernel/debug rw,nosuid,nodev,noexec,relatime shared:14 - debugfs debugfs rw > 29 14 0:12 / /sys/kernel/config rw,nosuid,nodev,noexec,relatime shared:15 - configfs configfs rw > 30 21 0:23 / /tmp rw,nosuid,nodev shared:16 - tmpfs tmpfs rw > 31 21 0:24 / /var/volatile rw,relatime shared:17 - tmpfs tmpfs rw > > 4. xino_auto > > 5. file in ro is correct > 6. file in rw is correct > 7. but the file in overlayfs is incorrect > > What's the reason for this? I suppose lower squashfs was recreated and mounted with an existing upper dir that already contains upper files with origin references to the old squashfs lower fs. > Can you give some help and suggestions to solve this problem? Cherry-pick this fix to your kernel: a888db310195 ovl: fix regression with re-formatted lower squashfs OR as a workaround, build the lower squashfs with the "-no-exports" mksquashfs option. Thanks, Amir.

2 3

Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
by Leon Romanovsky April 27, 2021

April 27, 2021

On Mon, Apr 26, 2021 at 11:22:54PM +0530, bkkarthik wrote: > On 21/04/26 03:50PM, Leon Romanovsky wrote: > > On Mon, Apr 26, 2021 at 02:00:58PM +0200, Rafael J. Wysocki wrote: > > > On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky <leon(a)kernel.org> wrote: > > > > > > > > On Fri, Apr 23, 2021 at 03:08:03PM -0600, Shuah Khan wrote: > > > > > On 4/22/21 12:03 PM, Anupama K Patil wrote: > > > > > > de, e are two variables of the type 'struct proc_dir_entry' > > > > > > which can be removed to save memory. This also fixes a coding style > > > > > > issue reported by checkpatch where we are suggested to make assignment > > > > > > outside the if statement. > > > > > > > > > > > > > > > > Sounds like a reasonable change. > > > > > > > > It is unclear how much changes to ISA code are welcomed. > > If changes to ISA code aren't welcomed, should these be marked obsolete in the MAINTIANERS file? I think so, but think that "Odd Fixes" better describes that Rafael wrote. Thanks

1 0

Re: [PATCH] drivers: pnp: proc.c: Removed unnecessary varibles
by Leon Romanovsky April 26, 2021

April 26, 2021

On Mon, Apr 26, 2021 at 02:00:58PM +0200, Rafael J. Wysocki wrote: > On Mon, Apr 26, 2021 at 6:57 AM Leon Romanovsky <leon(a)kernel.org> wrote: > > > > On Fri, Apr 23, 2021 at 03:08:03PM -0600, Shuah Khan wrote: > > > On 4/22/21 12:03 PM, Anupama K Patil wrote: > > > > de, e are two variables of the type 'struct proc_dir_entry' > > > > which can be removed to save memory. This also fixes a coding style > > > > issue reported by checkpatch where we are suggested to make assignment > > > > outside the if statement. > > > > > > > > > > Sounds like a reasonable change. > > > > It is unclear how much changes to ISA code are welcomed. > > Real fixes and obvious cleanups are, not much more than that. While first part is easy to determine, the second one is more blurry. > > > According to the Wikipedia, even Windows Vista disabled ISA PnP by default. > > https://en.wikipedia.org/wiki/Legacy_Plug_and_Play#Specifications > > It is indeed unclear how many systems with this interface still run > Linux, but as long as the code is in the tree, there's nothing wrong > with attempting to improve it. There's no assurance that all such > patches will be applied, though. > > Thanks!

2 1

Device driver .shutdown() VS .remove()
by Luca Ceresoli April 26, 2021

April 26, 2021

Hello, despite having been searching for documentation I couldn't find out the exact and detailed difference between the .shutdown() and .remove() calls in struct device_driver. My best understanding follows. Any amendment or clarification would be appreciated, and why not it might end up in improving the docs. The different intent is kind of clear: remove() is called when the device is disconnected or otherwise removed from the system, and also when the driver is removed (which can only happen by unloading a kernel module, I think). shutdown() is called during reboot and shutdown. remove() is _not_ called during shutdown and reboot. shutdown() is not called on device or driver removal. Things get more blurred when trying to understand what each of the two should do. Definitely both must stop the device from producing any events to the rest of the system: stop DMAs, stop anything that would trigger interrupts etc. remove() should also "turn off" the device, but it's probably a good idea to do in shutdown() as well in order to not leave peripherals unnecessarily powered while rebooting. remove() must definitely free any allocated resources, which would be useless in shutdown(). >From the above it looks like the shutdown() actions must be a subset of remove() actions. How much of my understanding is correct? Thanks. -- Luca

2 2